[shaswata56]

Fix RSA PKCS#1 v1.5 forgery via nested DigestAlgorithm garbage (CVE-2026-85393)

Summary

RSASSA-PKCS1-v1_5 verification accepted signatures whose decrypted block contains a structurally "valid" but non-canonical DigestInfo. forge's BER parser and asn1.validate() intentionally tolerate malformed encodings, which leaves attacker-controlled slack bytes inside the block — a NULL parameters field carrying arbitrary content, non-minimal long-form length octets, or extra/garbage elements in the nested SEQUENCEs. That slack is exactly what a low-exponent (e.g. e = 3) PKCS#1 v1.5 forgery needs to control the digest bytes while keeping validation happy.

Approach

After asn1.validate() confirms the DigestInfo structure, the expected DigestInfo is re-encoded canonically from the parsed OID and digest (_encodeDigestInfo) and compared byte-for-byte against the decoded block. One comparison rejects every lenient-encoding variant at once — there is no enumeration of malformed shapes to get wrong.

Details:

• Both DigestAlgorithm forms that occur in practice are accepted: with the empty NULL parameters field (as encoded by forge and OpenSSL) and, for the algorithms that permit it, without it — selected by whether the validator captured a parameters element.
• The comparison is a prefix comparison so the internal _parseAllDigestBytes testing flag keeps tolerating trailing bytes; in the default mode asn1.fromDer() has already rejected them, making this an exact match.
• This replaces the element-count check from the first revision of this PR, which @matthewscottconroy and @subvincdebian showed was bypassable with count-preserving variants (NULL-with-content, long-form length octets) — see the review discussion. Canonical comparison is the approach proposed in Fix CVE-2026-85393: reject non-canonical DigestInfo in RSA PKCS#1 v1.5 verification #1159.

Tests

• tests/unit/rsa.js: regression vectors for the count-preserving variants — NULL parameters with content bytes (100-byte payload), non-minimal long-form length octets on the inner/outer SEQUENCE, garbage element in the parameters slot — plus the original forged low-exponent vectors and positive controls (forge- and OpenSSL-encoded valid signatures).
• tests/pocs/cve-2026-85393.js: self-contained PoC demonstrating the forgery against the previous behavior.
• Full suite: 834 passing, 0 failing.

Credits & relationship to other PRs

• Supersedes Fix RSA PKCS#1 v1.5 forgery via nested DigestAlgorithm garbage #1152 (@Krysthyan) — the lib/rsa.js hardening there is reused with credit. This PR was opened fresh because Fix RSA PKCS#1 v1.5 forgery via nested DigestAlgorithm garbage #1152's branch has a stale base, a describe.only that masked the rest of the RSA suite, and a regression vector that errors at the wrong check when actually executed.
• The canonical re-encoding comparison follows the approach from @matthewscottconroy's Fix CVE-2026-85393: reject non-canonical DigestInfo in RSA PKCS#1 v1.5 verification #1159 and his review here; @subvincdebian independently confirmed the count-check bypasses for both e = 3 and e = 65537.
• Happy to consolidate: if maintainers prefer landing Fix CVE-2026-85393: reject non-canonical DigestInfo in RSA PKCS#1 v1.5 verification #1159, the PoC, positive controls, and the count-preserving regression vectors from this branch are available to cherry-pick — whichever lands, those cases need coverage.

[matthewscottconroy]

Thanks for adding the PoC script and the positive DigestInfo tests. Having those in the repo is useful.

A couple of things that should help whoever reviews this:

Overlap with #1152. The lib/rsa.js change here is the same as the one in #1152 (Krysthyan, opened 2026-09-09), comment text included. Both diffs produce the same file (a4264822b..430905451). If this builds on #1152, it would help to say so in the description. It might also be simpler to offer the PoC and the new tests to #1152, so maintainers have one fewer duplicate PR to sort through.

The count-only check leaves some cases open. Requiring the inner DigestAlgorithm SEQUENCE to have 2 elements (or 1) doesn't cover two variants, because both keep the element count valid:

NULL with content: forge's parser doesn't require a NULL to be empty, so the parameters field can carry attacker-chosen bytes (100 bytes is tested) while the inner SEQUENCE stays at exactly 2 elements.
Non-minimal length octets: long-form lengths on the inner or outer SEQUENCE also add slack without changing the count.
subvincdebian confirmed both independently on #1159 for e=3 and e=65537: comment. That comment also links a separate harness that tests these cases.

For transparency: I'm the author of #1159. It takes a different approach: it re-encodes the expected DigestInfo canonically from the parsed OID and digest, then requires a byte-for-byte match. That rejects all of these variants with one comparison. I'm not attached to which PR lands. Whichever one does, it would be good to include regression tests for the NULL-with-content and long-form-length cases.

[shaswata56]

Thanks for adding the PoC script and the positive DigestInfo tests. Having those in the repo is useful.

A couple of things that should help whoever reviews this:

Overlap with #1152. The lib/rsa.js change here is the same as the one in #1152 (Krysthyan, opened 2026-09-09), comment text included. Both diffs produce the same file (a4264822b..430905451). If this builds on #1152, it would help to say so in the description. It might also be simpler to offer the PoC and the new tests to #1152, so maintainers have one fewer duplicate PR to sort through.

The count-only check leaves some cases open. Requiring the inner DigestAlgorithm SEQUENCE to have 2 elements (or 1) doesn't cover two variants, because both keep the element count valid:

NULL with content: forge's parser doesn't require a NULL to be empty, so the parameters field can carry attacker-chosen bytes (100 bytes is tested) while the inner SEQUENCE stays at exactly 2 elements. Non-minimal length octets: long-form lengths on the inner or outer SEQUENCE also add slack without changing the count. subvincdebian confirmed both independently on #1159 for e=3 and e=65537: comment. That comment also links a separate harness that tests these cases.

For transparency: I'm the author of #1159. It takes a different approach: it re-encodes the expected DigestInfo canonically from the parsed OID and digest, then requires a byte-for-byte match. That rejects all of these variants with one comparison. I'm not attached to which PR lands. Whichever one does, it would be good to include regression tests for the NULL-with-content and long-form-length cases.

Thanks @matthewscottconroy — you're right on both counts, and the count-only check is now gone.

I reproduced both count-preserving bypasses locally against the previous patch using the
regression vectors from #1159's test suite: a NULL parameters field carrying content bytes
and non-minimal long-form length octets both verified as true. The element-count approach
was never going to enumerate its way out of a lenient BER parser — as you and @subvincdebian
demonstrated.

889cee0 replaces the count checks with the canonical re-encoding comparison from your review
(and #1159): after asn1.validate(), the expected DigestInfo is re-encoded from the parsed
OID and digest (_encodeDigestInfo) and compared byte-for-byte against the decoded bytes.
One small difference from #1159's implementation: the expected form is selected based on
whether the NULL parameters were captured (with-parameters vs. parameter-less), since a
captured parameters element means the bytes can't match the parameter-less re-encoding
anyway. Like #1159, the comparison is a prefix comparison so the legacy tolerance of the
internal _parseAllDigestBytes testing flag is preserved; in the default mode
asn1.fromDer() has already rejected trailing bytes, making it an exact match.

Regression tests added for all three count-preserving variants (NULL with content, long-form
length octets, garbage element in the parameters slot), alongside the existing forged-vector
and positive-control tests. Full node suite: 834 passing, 0 failing.

On #1152: the description has credited it since the PR was opened ("Supersedes #1152 — fix
logic reused with credit to @Krysthyan"), together with the reasons for opening a fresh PR
(stale base, a describe.only that masked the whole RSA suite, and a regression vector that
errors at the wrong check when actually executed). Happy to make that more prominent if it
reads as buried.

And agreed on consolidation: if maintainers would rather land #1159, no objection from me.
This branch's PoC, positive controls, and the regression vectors for the NULL-content and
long-form cases are available to cherry-pick wherever they're wanted — whichever PR lands,
those two cases need regression coverage.

[shaswata56]

Some ecosystem context that may help review, since I checked whether this lenience exists elsewhere: forged the same malformed-DigestInfo vectors (e=3, garbage element in the inner DigestAlgorithm SEQUENCE; NULL parameters carrying content bytes; garbage element in the outer SEQUENCE; non-minimal long-form outer length) against the other mainstream JS RSA verifiers —

• node:crypto (OpenSSL): rejects all four.
• jsrsasign 11.1.5: rejects all four (its ASN1HEX parsing is strict on these shapes).
• node-forge 1.4.0: accepts the first two — exactly the variants this PR's canonical re-encoding comparison rejects.

So forge appears to be the outlier on DigestInfo lenience among JS verifiers, which suggests the canonical-compare direction here matches where the rest of the ecosystem already is. Happy to share the vector-generation harness if useful.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.29%. Comparing base (7232404) to head (9842c3d).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1160      +/-   ##
==========================================
+ Coverage   80.25%   80.29%   +0.03%     
==========================================
  Files          42       42              
  Lines        9597     9605       +8     
==========================================
+ Hits         7702     7712      +10     
+ Misses       1895     1893       -2     

Files with missing lines	Coverage Δ
lib/rsa.js	78.97% <100.00%> (+0.90%)	⬆️

... 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...9842c3d. 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.

[d4v1dst]

Hi, any chance this can be merged soon?
This may solve a lot of issues.

[shaswata56]

The advisory for CVE-2026-85393 (GHSA-86w9-cpqp-85rv) has been public since Sep 3 with <= 1.4.0 affected and no patched release, and there are now six open PRs proposing fixes (#1157, #1158, #1159, #1160, #1161, #1162) — while downstream npm audit failures have started citing these PRs as the awaited fix. To make the eventual decision easier, here's the field:

Converged design — canonical re-encode + compare (build the expected DigestInfo with the library's own DER encoder, require the decrypted bytes to match):

• Fix CVE-2026-85393: reject non-canonical DigestInfo in RSA PKCS#1 v1.5 verification #1159 — first canonical-compare PR; contributed the regression vectors that break the simpler check designs.
• fix(rsa): reject garbage in nested DigestAlgorithm SEQUENCE #1160 (this PR) — same approach plus those vectors, an end-to-end e=3 forgery PoC, full suite at 834 green, and the ecosystem comparison (jsrsasign and node:crypto reject all four malformed vectors; forge is the outlier).
• Reject non-canonical RSA DigestInfo using the signing encoder #1162 — same approach, plus the refinement of sharing the encoder with the signing path.

Targeted element checks — #1157, #1158, #1161: subsets of canonical compare; #1159's vectors demonstrate bypasses of shape/count-only checks (non-empty NULL parameters, non-minimal lengths), which canonical compare rejects by construction.

Proposal: ship one of the canonical-compare PRs as 1.4.1. I'm happy to do the legwork either way — fold #1162's shared-encoder refactor into this branch (or rebase onto whichever PR is preferred) and assemble a combined regression suite from all six. @matthewscottconroy — you know these vectors best, would value your call on which candidate should land. @davidlehn — anything I can take off your plate to get a 1.4.1 out the door?

[davidlehn]

Thank you. The keyless e=3 forgery vector, the e=1 test approach and the PoC (now tests/pocs/ghsa-2g46-22mc-6gwp.js) are in node-forge 1.4.1. The commit lists you as co-author, and you're credited in GHSA-2g46-22mc-6gwp. Closing, as the fix landed in a combined security release.