[matthewscottconroy]

Fixes #1149 / CVE-2026-85393.

Context and relationship to existing work

Thank you for your time on this. I know #1149 is one of many demands on it and that several contributors have already put work in here, so I've tried to make this quick to evaluate and to be clear about where it fits.

This builds directly on the analysis in #1149 and on the fixes already proposed there: #1152 (which #1151 was closed in favor of) and #1158. I'm grateful for that prior work. While validating those approaches I found that the element-count strategy they share does not fully close the vulnerability, so this PR takes a different approach rather than re-proposing the same one.

All of the existing PRs require the inner DigestAlgorithm SEQUENCE to contain only the OID plus an optional NULL, verified by element count. But forge's parser does not require a NULL to be empty, so an attacker can keep the inner SEQUENCE at exactly two elements and smuggle arbitrary bytes inside the NULL parameters field. I verified that #1152's exact check accepts a DigestInfo whose NULL carries 100 attacker-controlled bytes, which is ample slack for the low-exponent (e=3) forgery. Non-minimal (long form) length octets slip through for the same reason. Those vectors stay open under a count-only check at any nesting level.

The fix

Instead of enumerating malformed shapes, verification re-encodes the expected DigestInfo canonically from the parsed algorithm OID and digest and requires the decoded bytes to match. Any slack — an extra element in either SEQUENCE, a NULL carrying content, non-minimal length octets, trailing bytes — changes the encoding and is rejected in a single comparison.

Both the with-parameters (NULL) and parameter-less forms are accepted, preserving the optional-parameters behavior established in #968. I verified interop against OpenSSL-generated signatures for SHA-256/384/512 with both e=65537 and e=3.

Tests

• Regression tests for each slack vector: an extra element in the inner SEQUENCE, a non-NULL second element, a NULL carrying content bytes, and long-form length octets.
• A positive test that a valid parameter-less DigestInfo still verifies.
• npm run test-node: 833 passing, 0 failing (4 pending). npx mocha tests/unit/rsa.js: 105 passing.
• No new lint errors in the touched files (the repo's lint job is currently disabled and failing on pre-existing issues, so I did not gate on it).

CHANGELOG

Adds a 1.4.1 Security entry. The GHSA ID is left as a placeholder for you to fill in.

Disclosure

In the interest of transparency: I used an AI assistant (Claude) to analyze the vulnerability, review the existing issue and PRs, implement this change, and write its tests. I have reviewed and verified all of it myself, including running the full suite and the OpenSSL interop checks, but I want to be upfront about how it was produced so you can weigh it accordingly.

I'm happy to adapt this — align it with #1152's structure if you prefer a smaller diff, or close it in favor of another PR if you'd rather fix the remaining gaps there. Either way, the NULL-with-content and long-form length cases are the ones I'd suggest making sure are covered in whatever lands.

🤖 Generated with Claude Code

[volmerf]

When will this fix be released?

[subvincdebian]

I independently validated the RSA-only change at db05494352dd3c9a5a07c5270333dcc78866cad2 against the published node-forge@1.4.0 Node implementation. The base lib/rsa.js matches the published file, so the backport applies without unrelated source changes.

Results on Node 24.19.0:

• The full Node unit suite at this pinned PR head passed: 833 passing, 4 pending, using Mocha 11.7.5 with NODE_ENV=test and tests/unit/index.js. It still emits the existing deprecated Buffer() warning.
• A separate installed-dependency regression harness passed 30 tests after the backport. Before patching, 10 assertions failed because five malformed encodings were accepted for each exponent, 3 and 65537: an extra inner element, a non-NULL second element, a NULL containing 100 bytes, and non-minimal outer/inner length octets. All are rejected afterward with normal PKCS#1 type-1 padding; no _skipPaddingChecks flag is used.
• Canonical DigestInfo with and without NULL parameters, wrong-digest rejection, Node/OpenSSL SHA-256/384/512 signatures, RSA-PSS and NONE verification pass.
• The actual Expo consumers (@expo/cli@0.22.28 and @expo/code-signing-certificates@0.0.6) resolve the same patched implementation. Certificate/key PEM roundtrips, self-signed certificate validation, manifest signing plus independent Node verification, tampered-message rejection, and CSR verification pass. Expo iOS/Android/web JavaScript exports also pass.

The harness is available here: installed-dependency regressions. It would be useful to retain the non-minimal inner-length regression alongside the existing outer-length case. Counting nested elements alone does not address content inside NULL or non-minimal encodings.

Limits: the malformed signatures are constructed with a private key to exercise parser/verifier acceptance, rather than demonstrating a private-key-free forgery. I checked the Node implementation, not regenerated browser bundles. This validation is not a claim that every RSA attack has been excluded.

We kept the original package name/version and applied a pnpm patch; registry/Dependency Review advisories still flag 1.4.0. A maintainer-reviewed fixed release remains necessary for downstream consumers to clear those checks without exceptions. Thank you for proposing the canonical-encoding fix.

[matthewscottconroy]

Thanks for taking the time to do this, and especially for exercising it through the
real Expo consumers (@expo/cli, @expo/code-signing-certificates) rather than just
the unit suite, that's exactly the interop coverage I couldn't easily reproduce on my
end.

On your suggestion: both regressions you call out are already in this PR.
tests/unit/rsa.js adds cases for

• a NULL parameters field carrying content bytes, and
• non-minimal (long form) length octets on the inner and outer SEQUENCE,

alongside the extra-inner-element and non-NULL-second-element cases and a positive test
for the parameter-less DigestInfo form.

Your independent run is a useful confirmation of why the fix re-encodes rather than
counts elements: the NULL-with-content and non-minimal-length vectors you hit both keep
the inner SEQUENCE at exactly two elements, so a count-only check accepts them.
Rebuilding the canonical DigestInfo and comparing bytes is what closes them without
special-casing each shape.

Agreed that a local patch doesn't clear the registry / Dependency Review advisories,
that needs a maintainer-reviewed release. I've tried to keep this PR easy to review
(RSA-only diff, no changes to the shared ASN.1 parsing, existing error message
preserved) so that's as low-effort as possible whenever a maintainer has the bandwidth.
Thanks again.

[codecov-commenter]

Codecov Report

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1159      +/-   ##
==========================================
- Coverage   80.25%   80.24%   -0.02%     
==========================================
  Files          42       42              
  Lines        9597     9607      +10     
==========================================
+ Hits         7702     7709       +7     
- Misses       1895     1898       +3     

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

... 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...db05494. 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]

Thank you. This PR's encode-and-compare approach is the basis of the fix in node-forge 1.4.1 (GHSA-2g46-22mc-6gwp, GHSA-6qrw-m8cv-hcxf). The commit lists you as co-author, and you're credited in the advisory. Closing, as the fix landed in a combined security release.