Skip to content

Reject nonempty DigestAlgorithm NULL parameters - #1157

Closed
itsalexfer wants to merge 2 commits into
digitalbazaar:mainfrom
itsalexfer:fix/reject-nonempty-digest-null
Closed

itsalexfer wants to merge 2 commits into
digitalbazaar:mainfrom
itsalexfer:fix/reject-nonempty-digest-null

Conversation

@itsalexfer

Copy link
Copy Markdown

Builds on #1152, with credit to @Krysthyan for its nested DigestAlgorithm element-count fix and @geo-chen for the original report.

This supplement also rejects primitive ASN.1 NULL parameters containing bytes, which must have empty contents. The new synthetic private-key fixtures demonstrate malformed-encoding acceptance, not forgery without a private key. Compatibility tests retain absent and empty NULL parameters, legacy BER verification, PSS, NONE, and native-generated signatures.

Tests: Node 24.14.1 with pinned upstream Mocha 5.2.0: 837 passing, 4 pending; RSA subset: 109 passing, 4 pending. Focused prior-code checks: 8 passing/1 expected failure; patched: 9 passing. Existing lint errors remain; the borrowed regression adds one unused-variable warning.

AI disclosure: Supplemental tests were generated by an AI agent using GPT-6 Astra (Max reasoning). The NULL guard already existed in our local Lerix mitigation. We are contributing it to support eventual adoption of an official release.

Cristhian Hernandez and others added 2 commits October 4, 2026 09:01
Require DigestAlgorithm SEQUENCE element count (OID + optional NULL) so
asn1.validate cannot ignore interior padding that bypasses CVE-2026-33894.

Co-authored-by: Cursor <cursoragent@cursor.com>
Supplement the nested element-count fix with strict NULL contents validation and compatibility tests.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.26%. Comparing base (7232404) to head (683ab33).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1157      +/-   ##
==========================================
+ Coverage   80.25%   80.26%   +0.01%     
==========================================
  Files          42       42              
  Lines        9597     9597              
==========================================
+ Hits         7702     7703       +1     
+ Misses       1895     1894       -1     
Files with missing lines Coverage Δ
lib/rsa.js 78.65% <ø> (+0.57%) ⬆️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7232404...683ab33. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@davidlehn

Copy link
Copy Markdown
Member

Thank you for the fix. CVE-2026-85393 is fixed in node-forge 1.4.1 (GHSA-2g46-22mc-6gwp). Checking element counts leaves other non-canonical encodings open (NULL parameters with content, non-minimal lengths), so the release re-encodes the DigestInfo and compares bytes instead. Closing.

@davidlehn davidlehn closed this Oct 27, 2026
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.

3 participants