Repository navigation
Conversation
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>
|
Hi @Krysthyan I can get behind this solution instead of the one I've made, since it's simpler and does not introduce the potential overhead of adding captureAsn1 to check the same thing. Have you run the rest of the suites just in case they also pass? @davidlehn can you check this? The CVE is blocking several pipelines as much as I can see in the mentions of #1149 I will close my PR in favor of this one. |
|
Would it be possible to review this PR? We have a vulnerability on our application due to it. |
osv-scanner flags node-forge@1.4.0 (CVE-2026-85393 / GHSA-86w9-cpqp-85rv, an incomplete fix for CVE-2026-33894) in the Identity Platform blocking function's dependency tree. node-forge is the latest published version, the advisory has no fixed release (upstream digitalbazaar/forge#1152 is unmerged), and the parent gcip-cloud-functions@0.2.1 still requires node-forge@^1.3.1, so there is no upgrade path. node-forge is pulled in only transitively and is never imported: the SDK runtime (gcip-cloud-functions/lib/**) contains no require('node-forge'), and the inbound Identity Platform event JWT is verified via jsonwebtoken (Node crypto). Rather than ship dormant vulnerable crypto, replace the unused implementation with a documented local no-op: declare node-forge as a direct file: dependency on vendor/node-forge (version 1.4.1, just past the affected <=1.4.0 range and still satisfying the SDK's ^1.3.1), so npm installs only this local copy and no vulnerable node-forge enters the tree or the committed lockfile. The function's own tests still pass, confirming node-forge is not exercised. Remove vendor/node-forge and this dependency once upstream ships a fixed release.
hono override 4.13.5 -> 4.13.7 (GHSA-hxh3-vqpv-xpqv boundary XSS). node-forge has no fixed release, so backport upstream digitalbazaar/forge#1152 as a pnpm patch (byte-identical to upstream post-image) with a patch-integrity test; dev-tooling-only reachability via @expo/cli.
ulises-jeremias
left a comment
ulises-jeremias
left a comment
There was a problem hiding this comment.
I checked the current PR head ceba344 before deciding whether to run a full test report. The branch is stale against main and still has describe.only('jsbn') in tests/unit/jsbn.js; consequently npm test on this head reports only 5 passing tests, not the full suite. The current base has describe('jsbn') and the newer test fix. The targeted RSA suite does pass (101 passing, 4 pending).
Please update this branch from current main and rerun npm test before treating the full-suite gate as passed. The newer #1158 branch is already based on the test-fixed main; I validated its full suite at 829 passing / 4 pending, though it still has two new lint errors noted in my review there.
6 participants
Summary
obj.value.length === 2on the outerDigestInfoSEQUENCE.asn1.validatestill ignores extra children inside the nestedDigestAlgorithmSEQUENCE, so attackers can stuff ~314 garbage bytes there and forge signatures for low public exponent keys (e.g.e=3).DigestAlgorithmSEQUENCE to contain only the OID plus an optional NULL (same pattern as the outer length check).Test plan
DigestAlgorithmchildren[OID, NULL, garbage]verified astruebefore the change; throws invalid DigestInfo after.should check nested DigestAlgorithm element countintests/unit/rsa.js.npx mocha tests/unit/rsa.js— 101 passing, 4 pending.Related: #1151 (same issue; this PR uses
obj.value[0].value.lengthinstead of addingcaptureAsn1).