Fix encoding/decoding of multi-byte first OID subidentifier#1148
Open
spokodev wants to merge 1 commit into
Open
Fix encoding/decoding of multi-byte first OID subidentifier#1148spokodev wants to merge 1 commit into
spokodev wants to merge 1 commit into
Conversation
`oidToDer` wrote the first subidentifier (40 * arc1 + arc2) as a single
raw byte, and `derToOid` read it back as a single byte. Per X.690 8.19
the first subidentifier is a base-128 multi-byte value like every other
arc, so it can exceed one byte whenever arc1 is 2 (arc2 is then
unbounded). Both directions silently corrupted such OIDs:
oidToDer('2.999.3') // '43703' (should be '883703')
derToOid(hexToBytes('883703')) // '3.16.55.3' (should be '2.999.3')
This affects registered arcs that appear in real certificates, e.g.
2.41.x (biometrics), 2.49.x (NATO), 2.51.x (GS1).
Encode the first subidentifier through the existing base-128 loop, and
on decode accumulate it across continuation bytes before splitting it
into the first two arcs. Output verified against OpenSSL `asn1parse` and
the X.690 8.19 rule across an OID fuzz.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
asn1.oidToDer/asn1.derToOidmis-handle the first OID subidentifier when it spans more than one byte. The first subidentifier encodes the first two arcs as40 * arc1 + arc2; per X.690 §8.19 it is a base-128 multi-byte value like every other subidentifier, so forarc1 === 2(wherearc2is unbounded) it routinely exceeds one byte. Both directions corrupt such OIDs:40*arc1 + arc2with a singleputByte, truncating values ≥ 256 and never setting the continuation bit (so values in 128–255 produce a byte a decoder reads as "more bytes follow").3).This is not limited to the
2.999example arc: registered arcs that appear in real certificates are affected, e.g. 2.41.x (biometrics, ISO/IEC JTC 1 SC 37), 2.49.x (NATO), 2.51.x (GS1). Any DER carrying such an OID is written wrong and read wrong.Fix
oidToDer: feed the first subidentifier through the existing base-128 emitter instead of a rawputByte.derToOid: accumulate the first subidentifier across continuation bytes like every other subidentifier, then split it (value < 80 → ⌊value/40⌋.value%40, else2.(value−80)).Verification
2.999.3↔883703,2.100↔8134); they fail onmainand pass with the fix.asn1parseand an independent X.690 §8.19 implementation across an OID fuzz (first-arc sweeps + random wide arcs): 0 mismatches, and 0 changes to previously-correct OIDs.asn1,oids,x509,pemsuites pass unchanged.The pre-existing >32-bit-arc limitation (the
0xffffffffguard /TODO) is unchanged and out of scope.