Skip to content

Fix IPv6 netmask parsing for prefixes that end mid-word - #167

Merged
Xor-el merged 1 commit into
masterfrom
refactorings/some-enhancements
Aug 8, 2026
Merged

Fix IPv6 netmask parsing for prefixes that end mid-word#167
Xor-el merged 1 commit into
masterfrom
refactorings/some-enhancements

Conversation

@Xor-el

@Xor-el Xor-el commented Aug 8, 2026

Copy link
Copy Markdown
Owner

TGeneralName.ParseIPv6Mask built the trailing partial word as $FFFF shr (16 - LBits), which sets the low bits rather than the leading ones. Any prefix length that is not a multiple of 16 therefore produced an inverted mask word: /47 encoded 0x7FFF where 0xFFFE was required, and /49 encoded 0x0001 instead of 0x8000. The full words ahead of it were already correct, so the resulting iPAddress GeneralName is wrong only in the boundary word — quiet enough to survive unnoticed.

It survived because every existing case used /48, /128 or a literal colon-form mask, all of which either skip the partial-word branch or bypass it entirely. Shift the other way and invert instead, and add /47 and /49 cases so the branch is actually exercised.

Also in this change:

  • TCryptoLibConfig.X509.AllowEmptyIssuerCert: RFC 5280 sec. 4.1.2.4 requires a non-empty issuer DN and the parser rejects an empty one, but profiles that use a self-signed certificate purely as an identity carrier place no requirement on the field, and such certificates are in circulation. The switch is off by default and read-side only — it gates the TTbsCertificateStructure parse constructor alone; the public constructor and both TBS generators stay unconditional. The new test asserts the parse round-trips the original DER byte-identically and re-runs the three rejection paths with the switch on to prove generation is still strict.

  • IAsn1TaggedObject.HasTagNo, for symmetry with the existing HasTag / HasTagClass / HasContextTag trio. No consumers yet.

  • Regression test for Pkcs12Store.GetCertificateAlias: 16 certificate entries, each asserted to map back to its own alias before and after a save/reload, plus an absent certificate yielding no alias. The lookup walks key and value together so it was never at risk of pairing them positionally; the test locks that in.

  • Rename the private TBigInteger.GetLowestSetBitMaskFirst parameter AFirstWordMaskX to AFirstWordMask. Cosmetic.

TGeneralName.ParseIPv6Mask built the trailing partial word as
$FFFF shr (16 - LBits), which sets the low bits rather than the leading
ones. Any prefix length that is not a multiple of 16 therefore produced
an inverted mask word: /47 encoded 0x7FFF where 0xFFFE was required, and
/49 encoded 0x0001 instead of 0x8000. The full words ahead of it were
already correct, so the resulting iPAddress GeneralName is wrong only in
the boundary word — quiet enough to survive unnoticed.

It survived because every existing case used /48, /128 or a literal
colon-form mask, all of which either skip the partial-word branch or
bypass it entirely. Shift the other way and invert instead, and add /47
and /49 cases so the branch is actually exercised.

Also in this change:

- TCryptoLibConfig.X509.AllowEmptyIssuerCert: RFC 5280 sec. 4.1.2.4
  requires a non-empty issuer DN and the parser rejects an empty one, but
  profiles that use a self-signed certificate purely as an identity
  carrier place no requirement on the field, and such certificates are in
  circulation. The switch is off by default and read-side only — it gates
  the TTbsCertificateStructure parse constructor alone; the public
  constructor and both TBS generators stay unconditional. The new test
  asserts the parse round-trips the original DER byte-identically and
  re-runs the three rejection paths with the switch on to prove
  generation is still strict.

- IAsn1TaggedObject.HasTagNo, for symmetry with the existing HasTag /
  HasTagClass / HasContextTag trio. No consumers yet.

- Regression test for Pkcs12Store.GetCertificateAlias: 16 certificate
  entries, each asserted to map back to its own alias before and after a
  save/reload, plus an absent certificate yielding no alias. The lookup
  walks key and value together so it was never at risk of pairing them
  positionally; the test locks that in.

- Rename the private TBigInteger.GetLowestSetBitMaskFirst parameter
  AFirstWordMaskX to AFirstWordMask. Cosmetic.
@Xor-el
Xor-el merged commit cb6fe6b into master Aug 8, 2026
15 checks passed
@Xor-el
Xor-el deleted the refactorings/some-enhancements branch August 8, 2026 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant