diff --git a/.claude/rules/test-verification.xml b/.claude/rules/test-verification.xml new file mode 100644 index 00000000..9c0bb67a --- /dev/null +++ b/.claude/rules/test-verification.xml @@ -0,0 +1,40 @@ + + + Wait for user test confirmation before pushing fixes + + Tests in this project run only in the example React Native app — the + assistant cannot execute them. After fixing a bug or adding a feature + that would normally be validated by running tests, commit locally and + WAIT for the user to manually run the test suite (`bun ios` / + `bun android` and exercise the relevant suite) and confirm it passes + before pushing to the remote. + + + After committing a fix that requires runtime validation, do NOT immediately `git push`. + Tell the user the commit is ready and ask them to run the test that exercises the fix. + Wait for explicit user confirmation ("tests pass", "all green", "ship it", etc.) before pushing. + If the user reports a failure, iterate locally with new commits — do NOT push interim fixes either. + Once the user confirms, batch-push all the validated commits in one `git push`. + + + The change touches C++ that only the example app can exercise. + The change modifies behavior that an existing example-app test asserts. + The change adds new example-app tests whose pass/fail is unknown. + A previous push had a confirmed test failure and you are pushing the followup. + + + The change is purely documentation, plan files, or `.claude/` config — no runtime impact. + The user explicitly says "push it" or "ship now" before running tests. + The branch has never been pushed and you are creating the first PR push (still ask first if any unverified runtime change is included). + + + The /pr and /commit skills are written for projects where the assistant + can run tests itself. This project's testing model puts that step on the + user. Pushing unverified fixes ships potentially broken code to the + remote, pollutes PR history with revert-style "fix the fix" commits, + and burns user trust. The cost of waiting is a single round-trip + message; the cost of not waiting is a force-push or noise on the PR. + + true + + diff --git a/example/ios/Podfile.lock b/example/ios/Podfile.lock index 938ef2c3..6db11893 100644 --- a/example/ios/Podfile.lock +++ b/example/ios/Podfile.lock @@ -2815,7 +2815,7 @@ SPEC CHECKSUMS: NitroMmkv: afbc5b2fbf963be567c6c545aa1efcf6a9cec68e NitroModules: 11bba9d065af151eae51e38a6425e04c3b223ff3 OpenSSL-Universal: 9110d21982bb7e8b22a962b6db56a8aa805afde7 - QuickCrypto: 51001a1827a20257b7c159d8c527306cdb578b5a + QuickCrypto: f8ed40d88a6dcacc4451d22004c2fd22b8d07f79 RCT-Folly: 846fda9475e61ec7bcbf8a3fe81edfcaeb090669 RCTDeprecation: c4b9e2fd0ab200e3af72b013ed6113187c607077 RCTRequired: e97dd5dafc1db8094e63bc5031e0371f092ae92a diff --git a/example/package.json b/example/package.json index 5e41426c..6feb6f42 100644 --- a/example/package.json +++ b/example/package.json @@ -40,7 +40,7 @@ "react-native-mmkv": "4.0.1", "react-native-nitro-modules": "0.33.2", "react-native-quick-base64": "2.2.2", - "react-native-quick-crypto": "1.1.0", + "react-native-quick-crypto": "workspace:*", "react-native-safe-area-context": "5.6.2", "react-native-screens": "4.18.0", "react-native-vector-icons": "10.3.0", diff --git a/example/src/tests/cipher/cipher_tests.ts b/example/src/tests/cipher/cipher_tests.ts index d681efda..0f276982 100644 --- a/example/src/tests/cipher/cipher_tests.ts +++ b/example/src/tests/cipher/cipher_tests.ts @@ -399,3 +399,96 @@ test(SUITE, 'GCM tampered auth tag throws error', () => { expect(() => decipher.final()).to.throw(); }); + +// --- setAAD byte-offset regression tests --- +// Pre-fix, setAAD passed `buffer.buffer` to native, ignoring byteOffset / +// byteLength on sliced Buffers. That meant a sliced AAD authenticated the +// wrong bytes — a silent AEAD integrity violation. + +test( + SUITE, + 'GCM setAAD with sliced Buffer authenticates the slice (not backing)', + () => { + const testKey = Buffer.from(randomFillSync(new Uint8Array(32))); + const testIv = randomFillSync(new Uint8Array(12)); + const testPlaintext = Buffer.from('test data for AAD slice'); + + // Build a backing buffer with a known 16-byte AAD region in the middle and + // distinct surrounding bytes. The cipher must only authenticate the slice. + const backing = Buffer.concat([ + Buffer.from('PREFIX_NOISE_'), + Buffer.from('aad-payload-1234'), // 16-byte AAD window + Buffer.from('_SUFFIX_NOISE'), + ]); + const aadSlice = backing.subarray(13, 13 + 16); + expect(aadSlice.byteLength).to.equal(16); + expect(aadSlice.toString('utf8')).to.equal('aad-payload-1234'); + + // Encrypt with the sliced AAD. + const cipher = createCipheriv('aes-256-gcm', testKey, Buffer.from(testIv)); + cipher.setAAD(aadSlice); + const encrypted = Buffer.concat([ + cipher.update(testPlaintext), + cipher.final(), + ]); + const authTag = cipher.getAuthTag(); + + // Decrypt with a freshly-constructed Buffer carrying the same 16 logical + // bytes — no surrounding noise, byteOffset = 0. If setAAD honors the + // slice on encrypt, this must verify successfully. + const aadStandalone = Buffer.from('aad-payload-1234'); + const decipher = createDecipheriv( + 'aes-256-gcm', + testKey, + Buffer.from(testIv), + ); + decipher.setAAD(aadStandalone); + decipher.setAuthTag(authTag); + const plaintextOut = Buffer.concat([ + decipher.update(encrypted), + decipher.final(), + ]); + expect(plaintextOut.toString('utf8')).to.equal( + testPlaintext.toString('utf8'), + ); + }, +); + +test( + SUITE, + 'GCM setAAD with sliced Buffer rejects wrong AAD on decrypt', + () => { + // Mirror of the previous test but supplies different AAD bytes on decrypt + // — must fail authentication. + const testKey = Buffer.from(randomFillSync(new Uint8Array(32))); + const testIv = randomFillSync(new Uint8Array(12)); + const testPlaintext = Buffer.from('test data for AAD slice'); + + const backing = Buffer.concat([ + Buffer.from('PREFIX_NOISE_'), + Buffer.from('aad-payload-1234'), + Buffer.from('_SUFFIX_NOISE'), + ]); + const aadSlice = backing.subarray(13, 13 + 16); + + const cipher = createCipheriv('aes-256-gcm', testKey, Buffer.from(testIv)); + cipher.setAAD(aadSlice); + const encrypted = Buffer.concat([ + cipher.update(testPlaintext), + cipher.final(), + ]); + const authTag = cipher.getAuthTag(); + + // Decrypt with WRONG AAD bytes — must throw on final(). + const wrongAad = Buffer.from('aad-payload-DIFF'); + const decipher = createDecipheriv( + 'aes-256-gcm', + testKey, + Buffer.from(testIv), + ); + decipher.setAAD(wrongAad); + decipher.setAuthTag(authTag); + decipher.update(encrypted); + expect(() => decipher.final()).to.throw(); + }, +); diff --git a/example/src/tests/cipher/xsalsa20_tests.ts b/example/src/tests/cipher/xsalsa20_tests.ts index 8e6ba310..86d633f0 100644 --- a/example/src/tests/cipher/xsalsa20_tests.ts +++ b/example/src/tests/cipher/xsalsa20_tests.ts @@ -1,4 +1,10 @@ -import { Buffer, randomFillSync, xsalsa20 } from 'react-native-quick-crypto'; +import { + Buffer, + createCipheriv, + createDecipheriv, + randomFillSync, + xsalsa20, +} from 'react-native-quick-crypto'; import { expect } from 'chai'; import { test } from '../util'; @@ -24,3 +30,160 @@ test(SUITE, 'xsalsa20', () => { // test decrypted == data expect(decrypted).eql(data); }); + +// --- Streaming regression tests --- +// +// XSalsa20 is a stream cipher: chunked update() calls must advance the +// keystream, NOT restart it from block 0 every time. The previous +// implementation called crypto_stream_xor() on each update(), which restarted +// the keystream and produced a two-time pad if the caller streamed >1 chunk. +// +// These tests pin that fix in place by checking streaming equivalence with +// the one-shot xsalsa20() function, which is the correct reference output. + +const STREAM_KEY = Buffer.from( + 'a8a7d6a5d4a3d2a1a09f9e9d9c8b8a89a8a7d6a5d4a3d2a1a09f9e9d9c8b8a89', + 'hex', +); +const STREAM_NONCE = Buffer.from( + '111213141516171821222324252627283132333435363738', + 'hex', +); + +// Block-aligned split: two 64-byte chunks (full Salsa20 blocks). +test(SUITE, 'xsalsa20 streaming equivalence — block-aligned split', () => { + const data = Buffer.alloc(128); + for (let i = 0; i < data.length; i++) data[i] = i & 0xff; + + const oneShot = xsalsa20( + new Uint8Array(STREAM_KEY), + new Uint8Array(STREAM_NONCE), + new Uint8Array(data), + ); + + const cipher = createCipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE); + const part1 = cipher.update(data.subarray(0, 64)); + const part2 = cipher.update(data.subarray(64)); + const streamed = Buffer.concat([part1, part2, cipher.final()]); + + expect(new Uint8Array(streamed)).eql(oneShot); +}); + +// Mid-block split: 30 + 70 bytes, neither chunk is a multiple of 64. +test(SUITE, 'xsalsa20 streaming equivalence — mid-block split', () => { + const data = Buffer.alloc(100); + for (let i = 0; i < data.length; i++) data[i] = (i * 7 + 3) & 0xff; + + const oneShot = xsalsa20( + new Uint8Array(STREAM_KEY), + new Uint8Array(STREAM_NONCE), + new Uint8Array(data), + ); + + const cipher = createCipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE); + const part1 = cipher.update(data.subarray(0, 30)); + const part2 = cipher.update(data.subarray(30)); + const streamed = Buffer.concat([part1, part2, cipher.final()]); + + expect(new Uint8Array(streamed)).eql(oneShot); +}); + +// Many small chunks crossing several block boundaries. +test(SUITE, 'xsalsa20 streaming equivalence — many small chunks', () => { + const data = Buffer.alloc(257); + for (let i = 0; i < data.length; i++) data[i] = (i * 13 + 5) & 0xff; + + const oneShot = xsalsa20( + new Uint8Array(STREAM_KEY), + new Uint8Array(STREAM_NONCE), + new Uint8Array(data), + ); + + const cipher = createCipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE); + const chunkSizes = [1, 7, 16, 31, 33, 64, 65, 40]; + const parts: Buffer[] = []; + let offset = 0; + for (const size of chunkSizes) { + const end = Math.min(offset + size, data.length); + if (end > offset) parts.push(cipher.update(data.subarray(offset, end))); + offset = end; + } + if (offset < data.length) parts.push(cipher.update(data.subarray(offset))); + parts.push(cipher.final()); + const streamed = Buffer.concat(parts); + + expect(new Uint8Array(streamed)).eql(oneShot); +}); + +// Regression: identical plaintext in two consecutive update() calls MUST +// produce different ciphertexts because the keystream advances. The previous +// (buggy) implementation reset the keystream on every update(), so both +// chunks would have been bitwise identical — a two-time-pad break. +test(SUITE, 'xsalsa20 keystream advances across update() calls', () => { + const block = Buffer.alloc(64, 0xaa); + + const cipher = createCipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE); + const c1 = cipher.update(block); + const c2 = cipher.update(block); + cipher.final(); + + expect(c1.length).to.equal(block.length); + expect(c2.length).to.equal(block.length); + // If the bug returns, c1 === c2 (catastrophic). + expect(c1.equals(c2)).to.equal(false); +}); + +// Edge case: a chunk that exactly drains the leftover keystream to the block +// boundary, followed by a subsequent update. Catches a regression where +// `leftover_offset` doesn't wrap to the sentinel correctly. +test( + SUITE, + 'xsalsa20 streaming equivalence — drain-to-boundary then continue', + () => { + // 60 + 4 + 100 = 164 bytes. After the 60-byte chunk, leftover_offset=60; + // the 4-byte chunk drains exactly to 64 (sentinel); the 100-byte chunk + // must then start cleanly on a fresh block boundary. + const data = Buffer.alloc(164); + for (let i = 0; i < data.length; i++) data[i] = (i * 5 + 19) & 0xff; + + const oneShot = xsalsa20( + new Uint8Array(STREAM_KEY), + new Uint8Array(STREAM_NONCE), + new Uint8Array(data), + ); + + const cipher = createCipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE); + const part1 = cipher.update(data.subarray(0, 60)); + const part2 = cipher.update(data.subarray(60, 64)); + const part3 = cipher.update(data.subarray(64)); + const streamed = Buffer.concat([part1, part2, part3, cipher.final()]); + + expect(new Uint8Array(streamed)).eql(oneShot); + }, +); + +// Streaming round-trip: encrypt and decrypt streamed across multiple +// update() calls. Decryption is just XOR with the same keystream, so this +// also exercises the streaming state on the decrypt side. +test(SUITE, 'xsalsa20 streaming round-trip across two cipher instances', () => { + const data = Buffer.alloc(200); + for (let i = 0; i < data.length; i++) data[i] = (i * 11 + 17) & 0xff; + + const enc = createCipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE); + const ciphertext = Buffer.concat([ + enc.update(data.subarray(0, 50)), + enc.update(data.subarray(50, 130)), + enc.update(data.subarray(130)), + enc.final(), + ]); + + const dec = createDecipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE); + const decrypted = Buffer.concat([ + dec.update(ciphertext.subarray(0, 17)), + dec.update(ciphertext.subarray(17, 99)), + dec.update(ciphertext.subarray(99)), + dec.final(), + ]); + + expect(decrypted.equals(data)).to.equal(true); +}); diff --git a/example/src/tests/dh/dh_tests.ts b/example/src/tests/dh/dh_tests.ts index a15e0b20..5430736e 100644 --- a/example/src/tests/dh/dh_tests.ts +++ b/example/src/tests/dh/dh_tests.ts @@ -144,3 +144,45 @@ test(SUITE, 'verifyError should return 0 for created DH', () => { const dh = crypto.createDiffieHellman(prime, 2); assert.strictEqual(dh.verifyError, 0); }); + +// --- Peer public-key validation (security audit Phase 0.3) --- +// +// Without an explicit DH_check_pub_key call, EVP_PKEY_derive_set_peer +// silently accepts a peer pubkey of 0, 1, or p-1 and produces a +// "shared secret" of 0, 1, or +/-1 — the small-subgroup attack. + +test(SUITE, 'computeSecret should reject peer public key of 0', () => { + const alice = crypto.getDiffieHellman('modp14'); + alice.generateKeys(); + assert.throws(() => { + alice.computeSecret(Buffer.from([0])); + }, /too small/i); +}); + +test(SUITE, 'computeSecret should reject peer public key of 1', () => { + const alice = crypto.getDiffieHellman('modp14'); + alice.generateKeys(); + assert.throws(() => { + alice.computeSecret(Buffer.from([1])); + }, /too small/i); +}); + +test(SUITE, 'computeSecret should reject peer public key of p-1', () => { + const alice = crypto.getDiffieHellman('modp14'); + alice.generateKeys(); + // modp14 prime ends in 0xFF...FF, so p-1 differs only in the trailing byte. + const pMinus1 = Buffer.from(MODP14_PRIME, 'hex'); + pMinus1[pMinus1.length - 1] = pMinus1[pMinus1.length - 1]! ^ 0x01; + assert.throws(() => { + alice.computeSecret(pMinus1); + }, /too large/i); +}); + +test(SUITE, 'computeSecret should reject peer public key equal to p', () => { + const alice = crypto.getDiffieHellman('modp14'); + alice.generateKeys(); + const p = Buffer.from(MODP14_PRIME, 'hex'); + assert.throws(() => { + alice.computeSecret(p); + }, /too large|invalid/i); +}); diff --git a/example/src/tests/ecdh/ecdh_tests.ts b/example/src/tests/ecdh/ecdh_tests.ts index 75902fe8..bd8f7cd7 100644 --- a/example/src/tests/ecdh/ecdh_tests.ts +++ b/example/src/tests/ecdh/ecdh_tests.ts @@ -170,6 +170,75 @@ test(SUITE, 'should compute secret with sliced public key buffer', () => { ); }); +// --- Peer public-key validation (security audit Phase 0.3) --- +// +// Without an explicit point-on-curve check, an attacker can mount an +// invalid-curve attack: send a point on a related, weaker curve and recover +// bits of the victim's private key from the resulting "shared secret". + +test(SUITE, 'computeSecret should reject empty peer key (malformed)', () => { + const alice = crypto.createECDH('prime256v1'); + alice.generateKeys(); + assert.throws(() => { + alice.computeSecret(Buffer.alloc(0)); + }, /malformed|peer/i); +}); + +test(SUITE, 'computeSecret should reject the identity point', () => { + const alice = crypto.createECDH('prime256v1'); + alice.generateKeys(); + // SEC1 encodes the point at infinity as a single 0x00 octet. + assert.throws(() => { + alice.computeSecret(Buffer.from([0x00])); + }, /identity|malformed|peer/i); +}); + +test(SUITE, 'computeSecret should reject peer key with wrong length', () => { + const alice = crypto.createECDH('prime256v1'); + alice.generateKeys(); + // 64 random bytes — not a valid uncompressed P-256 point (would need 65). + assert.throws(() => { + alice.computeSecret(Buffer.alloc(64, 0xab)); + }, /malformed|peer/i); +}); + +test( + SUITE, + 'computeSecret should reject peer key from a different curve', + () => { + // Send a P-384 (97-byte) public key to a P-256 (65-byte) instance — the + // length and/or coordinates won't match the configured curve. + const alice = crypto.createECDH('prime256v1'); + alice.generateKeys(); + const evil = crypto.createECDH('secp384r1'); + evil.generateKeys(); + assert.throws(() => { + alice.computeSecret(evil.getPublicKey()); + }, /malformed|not on the configured curve|peer/i); + }, +); + +test( + SUITE, + 'computeSecret should reject point not on the configured curve', + () => { + // Take a valid P-256 pubkey and flip a bit in the y-coordinate. + // The decoded (x, y) won't satisfy y^2 = x^3 + ax + b on P-256, so + // EC_POINT_is_on_curve must reject it. + const alice = crypto.createECDH('prime256v1'); + alice.generateKeys(); + const bob = crypto.createECDH('prime256v1'); + bob.generateKeys(); + const pub = Buffer.from(bob.getPublicKey() as Buffer); + // Flip a bit in the last byte (y-coordinate LSB) — overwhelmingly unlikely + // to land on the curve again. + pub[pub.length - 1] = pub[pub.length - 1]! ^ 0x01; + assert.throws(() => { + alice.computeSecret(pub); + }, /not on the configured curve|malformed|peer/i); + }, +); + test(SUITE, 'getCurves - should return array of supported curves', () => { const curves = getCurves(); assert.isArray(curves); diff --git a/example/src/tests/keys/public_cipher.ts b/example/src/tests/keys/public_cipher.ts index dd827a67..72bc704f 100644 --- a/example/src/tests/keys/public_cipher.ts +++ b/example/src/tests/keys/public_cipher.ts @@ -1069,3 +1069,208 @@ test(SUITE, 'publicEncrypt/privateDecrypt are inverses', () => { expect(decrypted.toString()).to.equal(shortPlaintext.toString()); }); + +// --- Bleichenbacher oracle mitigation (security audit Phase 0.4) --- +// +// PKCS#1 v1.5 RSA decryption must NOT reveal whether a ciphertext had valid +// padding. We close the oracle two ways: +// (1) OpenSSL implicit rejection — corrupted PKCS#1 v1.5 ciphertexts +// silently decrypt to deterministic random-looking bytes instead of +// throwing. +// (2) Opaque error messages — any decrypt failure (PKCS#1 v1.5, OAEP, or +// publicDecrypt/verify_recover) throws the same generic message, +// never leaking the underlying OpenSSL error reason. + +test( + SUITE, + 'Bleichenbacher: corrupted PKCS#1 v1.5 ciphertext does not throw', + () => { + const publicKey = createPublicKey({ + key: Buffer.from(spki), + format: 'der', + type: 'spki', + }); + const privateKey = createPrivateKey({ + key: Buffer.from(pkcs8), + format: 'der', + type: 'pkcs8', + }); + + const encrypted = publicEncrypt( + { key: publicKey, padding: constants.RSA_PKCS1_PADDING }, + shortPlaintext, + ); + + // Flip a bit in the middle of the ciphertext. With a Bleichenbacher + // mitigation in place, decryption returns random-looking bytes; without + // it, OpenSSL reports a padding error and we throw a distinguishable one. + const corrupted = Buffer.from(encrypted); + const corruptAt = Math.floor(corrupted.length / 2); + corrupted[corruptAt] = corrupted[corruptAt]! ^ 0x01; + + expect(() => { + privateDecrypt( + { key: privateKey, padding: constants.RSA_PKCS1_PADDING }, + corrupted, + ); + }).to.not.throw(); + }, +); + +test( + SUITE, + 'Bleichenbacher: corrupted PKCS#1 v1.5 outputs are deterministic and distinct', + () => { + const publicKey = createPublicKey({ + key: Buffer.from(spki), + format: 'der', + type: 'spki', + }); + const privateKey = createPrivateKey({ + key: Buffer.from(pkcs8), + format: 'der', + type: 'pkcs8', + }); + + const encrypted = publicEncrypt( + { key: publicKey, padding: constants.RSA_PKCS1_PADDING }, + shortPlaintext, + ); + + const corruptA = Buffer.from(encrypted); + const aAt = Math.floor(corruptA.length / 2); + corruptA[aAt] = corruptA[aAt]! ^ 0x01; + const corruptB = Buffer.from(encrypted); + const bAt = Math.floor(corruptB.length / 2) + 7; + corruptB[bAt] = corruptB[bAt]! ^ 0x80; + + const out1 = privateDecrypt( + { key: privateKey, padding: constants.RSA_PKCS1_PADDING }, + corruptA, + ); + const out2 = privateDecrypt( + { key: privateKey, padding: constants.RSA_PKCS1_PADDING }, + corruptA, + ); + const out3 = privateDecrypt( + { key: privateKey, padding: constants.RSA_PKCS1_PADDING }, + corruptB, + ); + + // Implicit rejection is deterministic: same (key, ciphertext) → same output. + expect(Buffer.compare(out1, out2)).to.equal(0); + // Different ciphertexts → different "implicit-rejection" outputs (so the + // value isn't a constant the attacker could equality-test against). + expect(Buffer.compare(out1, out3)).to.not.equal(0); + }, +); + +test(SUITE, 'Bleichenbacher: OAEP decrypt errors are opaque', async () => { + const publicKey = createPublicKey({ + key: Buffer.from(spki), + format: 'der', + type: 'spki', + }); + const privateKey = createPrivateKey({ + key: Buffer.from(pkcs8), + format: 'der', + type: 'pkcs8', + }); + + const encrypted = publicEncrypt( + { key: publicKey, oaepHash: 'SHA-256', oaepLabel: label }, + shortPlaintext, + ); + + // Decrypt with the wrong label — must throw, but the error must NOT name + // the specific OpenSSL failure reason ("oaep", "label", "padding", etc.). + let caught: Error | undefined; + try { + privateDecrypt( + { + key: privateKey, + oaepHash: 'SHA-256', + oaepLabel: Buffer.from('wrong'), + }, + encrypted, + ); + } catch (e) { + caught = e as Error; + } + expect(caught).to.be.instanceOf(Error); + expect(caught!.message).to.equal('privateDecrypt failed'); + expect(caught!.message.toLowerCase()).to.not.match( + /openssl|padding|oaep|label|mgf|digest|version|pkcs/, + ); +}); + +test( + SUITE, + 'Bleichenbacher: OAEP / PKCS#1 wrong-padding errors look identical', + () => { + // An attacker could distinguish padding modes if our error string differed + // by ciphertext shape. Decrypt the same garbage twice — once asking for + // OAEP, once asking for PKCS#1 v1.5 — and confirm the (PKCS#1) call no-ops + // via implicit rejection while the OAEP call throws the same opaque error + // we'd get from any other OAEP failure. + const privateKey = createPrivateKey({ + key: Buffer.from(pkcs8), + format: 'der', + type: 'pkcs8', + }); + + const garbage = Buffer.alloc(256, 0xab); // RSA-2048 modulus size + + // PKCS#1 v1.5: implicit rejection → no throw. + expect(() => { + privateDecrypt( + { key: privateKey, padding: constants.RSA_PKCS1_PADDING }, + garbage, + ); + }).to.not.throw(); + + // OAEP: throw, but ONLY the opaque message. + let caught: Error | undefined; + try { + privateDecrypt( + { key: privateKey, padding: constants.RSA_PKCS1_OAEP_PADDING }, + garbage, + ); + } catch (e) { + caught = e as Error; + } + expect(caught).to.be.instanceOf(Error); + expect(caught!.message).to.equal('privateDecrypt failed'); + }, +); + +test( + PRIVATE_CIPHER_SUITE, + 'Bleichenbacher: publicDecrypt errors are opaque', + () => { + const publicKey = createPublicKey({ + key: Buffer.from(spki), + format: 'der', + type: 'spki', + }); + + // Garbage in. publicDecrypt is signature verification with the public + // key (anyone can perform it) so this is not a Bleichenbacher target — + // either silently returning an empty buffer (the empty-plaintext + // recovery path may match) OR throwing an opaque error is acceptable. + // What we DO require: if we throw, the message must NOT leak OpenSSL + // error reasons. + let caught: Error | undefined; + try { + publicDecrypt(publicKey, Buffer.alloc(256, 0xcd)); + } catch (e) { + caught = e as Error; + } + if (caught !== undefined) { + expect(caught.message).to.equal('publicDecrypt failed'); + expect(caught.message.toLowerCase()).to.not.match( + /openssl|padding|pkcs|version|recover|verify/, + ); + } + }, +); diff --git a/example/src/tests/utils/utils_tests.ts b/example/src/tests/utils/utils_tests.ts index b905f1ab..ac209126 100644 --- a/example/src/tests/utils/utils_tests.ts +++ b/example/src/tests/utils/utils_tests.ts @@ -180,3 +180,57 @@ test(SUITE, 'timingSafeEqual should work for HMAC comparison use case', () => { expect(crypto.timingSafeEqual(hmac1, hmac2)).to.equal(true); expect(crypto.timingSafeEqual(hmac1, hmac3)).to.equal(false); }); + +// --- timingSafeEqual byte-offset regression tests --- +// These cover the bug where TypedArray / Buffer views over a larger backing +// were compared against the entire backing instead of the view's window. + +test( + SUITE, + 'timingSafeEqual respects byteOffset/byteLength on Uint8Array views', + () => { + // Two distinct backings whose middle 4 bytes are equal but surrounding + // bytes differ. Pre-fix this returned false (length mismatch on the + // backings) or compared the wrong bytes. + const backingA = new Uint8Array([ + 0xaa, 0xaa, 0xde, 0xad, 0xbe, 0xef, 0xaa, 0xaa, + ]); + const backingB = new Uint8Array([ + 0xbb, 0xbb, 0xde, 0xad, 0xbe, 0xef, 0xbb, 0xbb, + ]); + const viewA = new Uint8Array(backingA.buffer, 2, 4); + const viewB = new Uint8Array(backingB.buffer, 2, 4); + expect(crypto.timingSafeEqual(viewA, viewB)).to.equal(true); + }, +); + +test( + SUITE, + 'timingSafeEqual returns false when view contents differ even if backings happen to match', + () => { + // Same backing, two non-overlapping views of equal length but different + // contents — must compare false. + const backing = new Uint8Array([0x01, 0x02, 0x03, 0x04, 0x05, 0x06]); + const left = new Uint8Array(backing.buffer, 0, 3); // 01 02 03 + const right = new Uint8Array(backing.buffer, 3, 3); // 04 05 06 + expect(crypto.timingSafeEqual(left, right)).to.equal(false); + }, +); + +test( + SUITE, + 'timingSafeEqual throws on view-length mismatch (not backing-length mismatch)', + () => { + // A 4-byte view inside an 8-byte backing vs a 4-byte view inside a + // different 12-byte backing. Backings differ in length; views match. + // Must NOT throw — pre-fix this threw RangeError because the backing + // lengths were what got compared. + const backingA = new Uint8Array(8); + const backingB = new Uint8Array(12); + backingA.set([1, 2, 3, 4], 2); + backingB.set([1, 2, 3, 4], 5); + const viewA = new Uint8Array(backingA.buffer, 2, 4); + const viewB = new Uint8Array(backingB.buffer, 5, 4); + expect(crypto.timingSafeEqual(viewA, viewB)).to.equal(true); + }, +); diff --git a/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp b/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp index 703fffb8..5801c50f 100644 --- a/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp +++ b/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp @@ -25,6 +25,38 @@ int toOpenSSLPadding(int padding) { } } +// Bleichenbacher mitigation. For RSA PKCS#1 v1.5 decryption, ask OpenSSL to +// substitute random-looking plaintext on padding-check failure rather than +// surfacing a distinguishable error. This closes the "padding-valid / +// padding-invalid" oracle that the Million Message Attack depends on. The +// `EVP_PKEY_CTX_ctrl_str` knob was added in OpenSSL 3.2; if the underlying +// build does not support it (BoringSSL, older OpenSSL) we refuse to perform +// PKCS#1 v1.5 decryption rather than silently fall back to a configuration +// that leaves the timing-side oracle open. Node.js (`crypto_cipher.cc`) +// applies the same hard-fail policy. Returns true if implicit rejection is +// engaged or not applicable (OAEP); false if PKCS#1 v1.5 was requested but +// the knob failed. Always clears the OpenSSL error stack on failure so a +// rejected knob does not leak through to a later operation. +[[nodiscard]] static bool enableImplicitRejectionIfPkcs1(EVP_PKEY_CTX* ctx, int opensslPadding) { + if (opensslPadding != RSA_PKCS1_PADDING) { + return true; + } + bool ok = EVP_PKEY_CTX_ctrl_str(ctx, "rsa_pkcs1_implicit_rejection", "1") > 0; + if (!ok) { + ERR_clear_error(); + } + return ok; +} + +// Throw the SAME message regardless of the underlying OpenSSL error so that +// callers (and remote attackers in oracle-style scenarios) cannot distinguish +// "padding invalid" from "data too large", "bad version", "wrong key", etc. +// The OpenSSL error stack is cleared so it is not observable later. +[[noreturn]] static void throwOpaqueDecryptFailure() { + ERR_clear_error(); + throw std::runtime_error("RSA decryption failed"); +} + std::shared_ptr HybridRsaCipher::encrypt(const std::shared_ptr& keyHandle, const std::shared_ptr& data, double padding, const std::string& hashAlgorithm, @@ -147,6 +179,11 @@ std::shared_ptr HybridRsaCipher::decrypt(const std::shared_ptr= 3.2)"); + } + if (paddingInt == kRsaOaepPadding) { const EVP_MD* md = getDigestByName(hashAlgorithm); if (EVP_PKEY_CTX_set_rsa_oaep_md(ctx, md) <= 0) { @@ -180,23 +217,20 @@ std::shared_ptr HybridRsaCipher::decrypt(const std::shared_ptrdata(); size_t inlen = native_data->size(); + // Both decrypt calls below operate on attacker-controlled ciphertext, so + // any failure must be surfaced with an opaque, content-independent message. + // See enableImplicitRejectionIfPkcs1 / throwOpaqueDecryptFailure above. size_t outlen; if (EVP_PKEY_decrypt(ctx, nullptr, &outlen, in, inlen) <= 0) { EVP_PKEY_CTX_free(ctx); - unsigned long err = ERR_get_error(); - char err_buf[256]; - ERR_error_string_n(err, err_buf, sizeof(err_buf)); - throw std::runtime_error("Failed to determine output length: " + std::string(err_buf)); + throwOpaqueDecryptFailure(); } auto out_buf = std::make_unique(outlen); if (EVP_PKEY_decrypt(ctx, out_buf.get(), &outlen, in, inlen) <= 0) { EVP_PKEY_CTX_free(ctx); - unsigned long err = ERR_get_error(); - char err_buf[256]; - ERR_error_string_n(err, err_buf, sizeof(err_buf)); - throw std::runtime_error("Decryption failed: " + std::string(err_buf)); + throwOpaqueDecryptFailure(); } EVP_PKEY_CTX_free(ctx); @@ -239,13 +273,12 @@ std::shared_ptr HybridRsaCipher::publicDecrypt(const std::shared_pt const unsigned char* in = native_data->data(); size_t inlen = native_data->size(); + // verify_recover acts on attacker-controlled ciphertext too — surface only + // an opaque error so a remote caller cannot distinguish failure modes. size_t outlen; if (EVP_PKEY_verify_recover(ctx, nullptr, &outlen, in, inlen) <= 0) { EVP_PKEY_CTX_free(ctx); - unsigned long err = ERR_get_error(); - char err_buf[256]; - ERR_error_string_n(err, err_buf, sizeof(err_buf)); - throw std::runtime_error("Failed to determine output length: " + std::string(err_buf)); + throwOpaqueDecryptFailure(); } if (outlen == 0) { @@ -257,19 +290,27 @@ std::shared_ptr HybridRsaCipher::publicDecrypt(const std::shared_pt auto out_buf = std::make_unique(outlen); if (EVP_PKEY_verify_recover(ctx, out_buf.get(), &outlen, in, inlen) <= 0) { + // Empty-plaintext recovery: when the original message was zero bytes, + // OpenSSL's verify_recover surfaces a specific reason code rather than + // returning success+outlen=0. Match the narrow code from the original + // implementation and return an empty buffer so `publicDecrypt(privateEncrypt(""))` + // round-trips. publicDecrypt is signature verification with the PUBLIC + // key — anyone can perform it — so the special case does not enable a + // Bleichenbacher-style oracle. The fall-through still uses the opaque + // throw helper. + // + // Use ERR_get_error (oldest in the FIFO queue) to match the inner + // padding-check error rather than ERR_peek_last_error which returns + // the outer wrapper code that doesn't satisfy the narrow match. unsigned long err = ERR_get_error(); - char err_buf[256]; - ERR_error_string_n(err, err_buf, sizeof(err_buf)); - if ((err & 0xFFFFFFF) == 0x1C880004 || (err & 0xFF) == 0x04) { ERR_clear_error(); EVP_PKEY_CTX_free(ctx); uint8_t* empty_buf = new uint8_t[1]; return std::make_shared(empty_buf, 0, [empty_buf]() { delete[] empty_buf; }); } - EVP_PKEY_CTX_free(ctx); - throw std::runtime_error("Public decryption failed: " + std::string(err_buf)); + throwOpaqueDecryptFailure(); } EVP_PKEY_CTX_free(ctx); @@ -369,6 +410,11 @@ std::shared_ptr HybridRsaCipher::privateDecrypt(const std::shared_p throw std::runtime_error("Failed to set RSA padding"); } + if (!enableImplicitRejectionIfPkcs1(ctx, opensslPadding)) { + EVP_PKEY_CTX_free(ctx); + throw std::runtime_error("RSA PKCS#1 v1.5 decryption requires OpenSSL implicit-rejection support (>= 3.2)"); + } + if (paddingInt == kRsaOaepPadding) { const EVP_MD* md = getDigestByName(hashAlgorithm); if (EVP_PKEY_CTX_set_rsa_oaep_md(ctx, md) <= 0) { @@ -402,23 +448,20 @@ std::shared_ptr HybridRsaCipher::privateDecrypt(const std::shared_p const unsigned char* in = native_data->data(); size_t inlen = native_data->size(); + // Both decrypt calls below operate on attacker-controlled ciphertext, so + // any failure must be surfaced with an opaque, content-independent message. + // See enableImplicitRejectionIfPkcs1 / throwOpaqueDecryptFailure above. size_t outlen; if (EVP_PKEY_decrypt(ctx, nullptr, &outlen, in, inlen) <= 0) { EVP_PKEY_CTX_free(ctx); - unsigned long err = ERR_get_error(); - char err_buf[256]; - ERR_error_string_n(err, err_buf, sizeof(err_buf)); - throw std::runtime_error("Failed to determine output length: " + std::string(err_buf)); + throwOpaqueDecryptFailure(); } auto out_buf = std::make_unique(outlen); if (EVP_PKEY_decrypt(ctx, out_buf.get(), &outlen, in, inlen) <= 0) { EVP_PKEY_CTX_free(ctx); - unsigned long err = ERR_get_error(); - char err_buf[256]; - ERR_error_string_n(err, err_buf, sizeof(err_buf)); - throw std::runtime_error("Private decryption failed: " + std::string(err_buf)); + throwOpaqueDecryptFailure(); } EVP_PKEY_CTX_free(ctx); diff --git a/packages/react-native-quick-crypto/cpp/cipher/XSalsa20Cipher.cpp b/packages/react-native-quick-crypto/cpp/cipher/XSalsa20Cipher.cpp index 1fd0b353..ce83bf3a 100644 --- a/packages/react-native-quick-crypto/cpp/cipher/XSalsa20Cipher.cpp +++ b/packages/react-native-quick-crypto/cpp/cipher/XSalsa20Cipher.cpp @@ -1,4 +1,6 @@ +#include #include // For std::memcpy +#include // For std::unique_ptr #include // For std::runtime_error #include "NitroModules/ArrayBuffer.hpp" @@ -28,11 +30,27 @@ void XSalsa20Cipher::init(const std::shared_ptr cipher_key, const s // Copy key and nonce data std::memcpy(key, native_key->data(), crypto_stream_KEYBYTES); std::memcpy(nonce, native_iv->data(), crypto_stream_NONCEBYTES); + + // Reset streaming state so a re-init'd cipher does not accidentally reuse + // keystream bytes from a previous session. + block_counter = 0; + leftover_offset = kSalsa20BlockBytes; + is_finalized = false; } /** - * xsalsa20 call to sodium implementation + * xsalsa20 update — encrypts/decrypts `data` while keeping the keystream + * advancing across successive update() calls. + * + * Implementation notes: + * 1. First, drain any unused keystream bytes left over from the previous + * chunk's trailing partial block. + * 2. Then process as many aligned whole 64-byte blocks as possible by + * jumping the keystream to `block_counter` via crypto_stream_xsalsa20_xor_ic. + * 3. For the remaining tail (< 64 bytes), generate one full keystream + * block, XOR the requested prefix, and stash the unused suffix for the + * next update() call. */ std::shared_ptr XSalsa20Cipher::update(const std::shared_ptr& data) { checkNotFinalized(); @@ -40,12 +58,60 @@ std::shared_ptr XSalsa20Cipher::update(const std::shared_ptrsize()]; - int result = crypto_stream_xor(output, native_data->data(), native_data->size(), nonce, key); - if (result != 0) { - throw std::runtime_error("XSalsa20Cipher: Failed to update"); + const std::size_t data_size = native_data->size(); + + if (data_size == 0) { + return std::make_shared(nullptr, 0, nullptr); + } + + // Owning buffer: prevents leaking `output` if we throw on the way out. + auto output = std::make_unique(data_size); + const uint8_t* input = native_data->data(); + std::size_t pos = 0; + + // (1) Drain any unused keystream from the previous update()'s tail block. + if (leftover_offset < kSalsa20BlockBytes) { + const std::size_t avail = kSalsa20BlockBytes - leftover_offset; + const std::size_t take = std::min(avail, data_size); + for (std::size_t i = 0; i < take; ++i) { + output[i] = input[i] ^ leftover_keystream[leftover_offset + i]; + } + leftover_offset += take; + pos = take; } - return std::make_shared(output, native_data->size(), [=]() { delete[] output; }); + + // (2) Encrypt the aligned whole blocks at the current block counter. + const std::size_t remaining = data_size - pos; + const std::size_t whole_blocks = remaining / kSalsa20BlockBytes; + const std::size_t whole_bytes = whole_blocks * kSalsa20BlockBytes; + if (whole_bytes > 0) { + int rc = crypto_stream_xsalsa20_xor_ic(output.get() + pos, input + pos, whole_bytes, nonce, block_counter, key); + if (rc != 0) { + throw std::runtime_error("XSalsa20Cipher: crypto_stream_xsalsa20_xor_ic failed"); + } + block_counter += whole_blocks; + pos += whole_bytes; + } + + // (3) For any trailing partial block, generate one full keystream block, + // XOR the requested prefix, and stash the unused keystream bytes for + // the next update() call. + const std::size_t tail = data_size - pos; + if (tail > 0) { + uint8_t zeros[kSalsa20BlockBytes] = {}; + int rc = crypto_stream_xsalsa20_xor_ic(leftover_keystream, zeros, kSalsa20BlockBytes, nonce, block_counter, key); + if (rc != 0) { + throw std::runtime_error("XSalsa20Cipher: crypto_stream_xsalsa20_xor_ic failed"); + } + for (std::size_t i = 0; i < tail; ++i) { + output[pos + i] = input[pos + i] ^ leftover_keystream[i]; + } + leftover_offset = tail; + block_counter += 1; + } + + uint8_t* raw = output.release(); + return std::make_shared(raw, data_size, [=]() { delete[] raw; }); #endif } diff --git a/packages/react-native-quick-crypto/cpp/cipher/XSalsa20Cipher.hpp b/packages/react-native-quick-crypto/cpp/cipher/XSalsa20Cipher.hpp index a4d8bba1..098a5f96 100644 --- a/packages/react-native-quick-crypto/cpp/cipher/XSalsa20Cipher.hpp +++ b/packages/react-native-quick-crypto/cpp/cipher/XSalsa20Cipher.hpp @@ -8,6 +8,9 @@ #define crypto_stream_NONCEBYTES 24 // XSalsa20 nonce size (24 bytes) #endif +#include +#include + #include "HybridCipher.hpp" #include "NitroModules/ArrayBuffer.hpp" @@ -26,8 +29,21 @@ class XSalsa20Cipher : public HybridCipher { std::shared_ptr final() override; private: + // Salsa20 (and therefore XSalsa20) processes the keystream in 64-byte blocks. + static constexpr std::size_t kSalsa20BlockBytes = 64; + uint8_t key[crypto_stream_KEYBYTES]; uint8_t nonce[crypto_stream_NONCEBYTES]; + + // Streaming state — keeps the keystream advancing across multiple update() + // calls. Without this, every update() would restart at block 0, producing + // identical keystream for each chunk (a two-time-pad break). + uint8_t leftover_keystream[kSalsa20BlockBytes] = {}; + // 0..kSalsa20BlockBytes; the sentinel value kSalsa20BlockBytes means "no + // leftover keystream available — start the next chunk on a block boundary". + std::size_t leftover_offset = kSalsa20BlockBytes; + // Index of the next 64-byte keystream block to consume. + uint64_t block_counter = 0; }; } // namespace margelo::nitro::crypto diff --git a/packages/react-native-quick-crypto/cpp/dh/HybridDiffieHellman.cpp b/packages/react-native-quick-crypto/cpp/dh/HybridDiffieHellman.cpp index 01fe7790..b0adab98 100644 --- a/packages/react-native-quick-crypto/cpp/dh/HybridDiffieHellman.cpp +++ b/packages/react-native-quick-crypto/cpp/dh/HybridDiffieHellman.cpp @@ -133,6 +133,35 @@ std::shared_ptr HybridDiffieHellman::computeSecret(const std::share const BIGNUM *p, *q, *g; DH_get0_pqg(ourDh, &p, &q, &g); + // Validate the peer's public key against our DH parameters BEFORE doing + // anything else. EVP_PKEY_derive_set_peer() does NOT call DH_check_pub_key, + // so without this check a peer key of 0, 1, or p-1 silently produces a + // degenerate "shared secret" (0, 1, or ±1) — the small-subgroup attack. + // Match the ncrypto pattern (DHPointer::checkPublicKey) and Node.js error + // surface so callers see why the key was rejected. + { + BN_ptr peerPubCheck(BN_bin2bn(otherPublicKey->data(), static_cast(otherPublicKey->size()), nullptr), BN_free); + if (!peerPubCheck) { + throw std::runtime_error("DiffieHellman: failed to parse peer public key"); + } + int codes = 0; + if (DH_check_pub_key(ourDh, peerPubCheck.get(), &codes) != 1) { + throw std::runtime_error("DiffieHellman: failed to check peer public key"); + } + if (codes & DH_CHECK_PUBKEY_TOO_SMALL) { + throw std::runtime_error("DiffieHellman: peer public key is too small (<= 1)"); + } + if (codes & DH_CHECK_PUBKEY_TOO_LARGE) { + throw std::runtime_error("DiffieHellman: peer public key is too large (>= p-1)"); + } + if (codes & DH_CHECK_PUBKEY_INVALID) { + throw std::runtime_error("DiffieHellman: peer public key is invalid (not in subgroup)"); + } + if (codes != 0) { + throw std::runtime_error("DiffieHellman: peer public key is invalid"); + } + } + // Create peer DH with same parameters but peer's public key DH_ptr peerDh(DH_new(), DH_free); if (!peerDh) { diff --git a/packages/react-native-quick-crypto/cpp/ecdh/HybridECDH.cpp b/packages/react-native-quick-crypto/cpp/ecdh/HybridECDH.cpp index b5c7a084..d567b411 100644 --- a/packages/react-native-quick-crypto/cpp/ecdh/HybridECDH.cpp +++ b/packages/react-native-quick-crypto/cpp/ecdh/HybridECDH.cpp @@ -65,6 +65,29 @@ std::shared_ptr HybridECDH::computeSecret(const std::shared_ptrdata(), otherPublicKey->size(), nullptr) != 1) { + throw std::runtime_error("ECDH: peer public key is malformed"); + } + if (EC_POINT_is_at_infinity(_group.get(), peerPoint.get())) { + throw std::runtime_error("ECDH: peer public key is the identity point"); + } + if (EC_POINT_is_on_curve(_group.get(), peerPoint.get(), nullptr) != 1) { + throw std::runtime_error("ECDH: peer public key is not on the configured curve"); + } + } + // Build peer EVP_PKEY from raw public key octets EVP_PKEY_ptr peerPkey(createEcEvpPkey(_curveName.c_str(), otherPublicKey->data(), otherPublicKey->size()), EVP_PKEY_free); diff --git a/packages/react-native-quick-crypto/src/cipher.ts b/packages/react-native-quick-crypto/src/cipher.ts index 0abb86bb..3492247e 100644 --- a/packages/react-native-quick-crypto/src/cipher.ts +++ b/packages/react-native-quick-crypto/src/cipher.ts @@ -211,7 +211,14 @@ class CipherCommon extends Stream.Transform { if (!this.native || typeof this.native.setAAD !== 'function') { throw new Error('Cipher native object or setAAD method not initialized.'); } - const res = this.native.setAAD(buffer.buffer, options?.plaintextLength); + // Use binaryLikeToArrayBuffer (not `buffer.buffer`) so that sliced / + // offset views send only the AAD bytes the caller intended. Passing the + // raw backing ArrayBuffer authenticates the wrong data and silently + // breaks the AEAD integrity guarantee. + const res = this.native.setAAD( + binaryLikeToArrayBuffer(buffer), + options?.plaintextLength, + ); if (!res) { throw new Error('setAAD failed (native call returned false)'); } diff --git a/packages/react-native-quick-crypto/src/keys/publicCipher.ts b/packages/react-native-quick-crypto/src/keys/publicCipher.ts index 68363ab7..584e8d76 100644 --- a/packages/react-native-quick-crypto/src/keys/publicCipher.ts +++ b/packages/react-native-quick-crypto/src/keys/publicCipher.ts @@ -144,8 +144,13 @@ export function publicDecrypt( paddingMode, ); return Buffer.from(decrypted); - } catch (error) { - throw new Error(`publicDecrypt failed: ${(error as Error).message}`); + } catch { + // Bleichenbacher mitigation: surface a single, content-independent error + // for every decrypt failure so an attacker cannot use error-message + // differences as a padding oracle. The native side already collapses its + // OpenSSL error codes to the same opaque message; we drop it here too + // rather than re-leaking it via string interpolation. + throw new Error('publicDecrypt failed'); } } @@ -244,7 +249,8 @@ export function privateDecrypt( oaepLabel, ); return Buffer.from(decrypted); - } catch (error) { - throw new Error(`privateDecrypt failed: ${(error as Error).message}`); + } catch { + // Bleichenbacher mitigation — see publicDecrypt above. + throw new Error('privateDecrypt failed'); } } diff --git a/packages/react-native-quick-crypto/src/utils/conversion.ts b/packages/react-native-quick-crypto/src/utils/conversion.ts index 4fddea8c..e8de35f5 100644 --- a/packages/react-native-quick-crypto/src/utils/conversion.ts +++ b/packages/react-native-quick-crypto/src/utils/conversion.ts @@ -7,12 +7,15 @@ import type { ABV, BinaryLikeNode, BufferLike } from './types'; const utils = NitroModules.createHybridObject('Utils'); /** - * Converts supplied argument to an ArrayBuffer. Note this does not copy the - * data so it is faster than toArrayBuffer. Not copying is important for - * functions like randomFill which need to be able to write to the underlying - * buffer. - * @param buf - * @returns ArrayBuffer + * Returns the underlying ArrayBuffer of a Buffer / TypedArray view **without + * copying**, ignoring `byteOffset`/`byteLength`. The full backing storage is + * exposed. + * + * Only use this when the caller separately tracks `byteOffset`/`byteLength` + * and the native receiver needs to write back into the original memory + * (e.g. `randomFill`). For data that will be read by native crypto, use + * `binaryLikeToArrayBuffer`/`toArrayBuffer` instead — those slice to the + * view's region and won't leak unrelated bytes from the backing buffer. */ export const abvToArrayBuffer = (buf: ABV) => { if (CraftzdogBuffer.isBuffer(buf)) { diff --git a/packages/react-native-quick-crypto/src/utils/timingSafeEqual.ts b/packages/react-native-quick-crypto/src/utils/timingSafeEqual.ts index 948ffa59..c63aa932 100644 --- a/packages/react-native-quick-crypto/src/utils/timingSafeEqual.ts +++ b/packages/react-native-quick-crypto/src/utils/timingSafeEqual.ts @@ -1,7 +1,7 @@ import { NitroModules } from 'react-native-nitro-modules'; import type { Utils } from '../specs/utils.nitro'; import type { ABV } from './types'; -import { abvToArrayBuffer } from './conversion'; +import { binaryLikeToArrayBuffer } from './conversion'; let utils: Utils; function getNative(): Utils { @@ -12,8 +12,13 @@ function getNative(): Utils { } export function timingSafeEqual(a: ABV, b: ABV): boolean { - const bufA = abvToArrayBuffer(a); - const bufB = abvToArrayBuffer(b); + // Use binaryLikeToArrayBuffer (not abvToArrayBuffer) so that TypedArray / + // Buffer views are sliced to their `byteOffset`/`byteLength` window. The + // zero-copy `abvToArrayBuffer` returns the entire backing buffer, which + // would (a) compare unrelated bytes and (b) silently fail the byte-length + // check for any view smaller than its backing. + const bufA = binaryLikeToArrayBuffer(a); + const bufB = binaryLikeToArrayBuffer(b); if (bufA.byteLength !== bufB.byteLength) { throw new RangeError('Input buffers must have the same byte length'); diff --git a/packages/react-native-quick-crypto/src/x509certificate.ts b/packages/react-native-quick-crypto/src/x509certificate.ts index 318b05e7..b654fad5 100644 --- a/packages/react-native-quick-crypto/src/x509certificate.ts +++ b/packages/react-native-quick-crypto/src/x509certificate.ts @@ -78,12 +78,11 @@ export class X509Certificate { 'X509CertificateHandle', ); - let ab: ArrayBuffer; - if (typeof buffer === 'string') { - ab = Buffer.from(buffer).buffer as ArrayBuffer; - } else { - ab = binaryLikeToArrayBuffer(buffer); - } + // For string input, route through binaryLikeToArrayBuffer so the result + // is a tight ArrayBuffer of just the encoded bytes. `Buffer.from(str).buffer` + // can return a pool-backed ArrayBuffer with byteOffset > 0, exposing + // unrelated bytes (and giving the wrong byteLength) to native. + const ab: ArrayBuffer = binaryLikeToArrayBuffer(buffer); this.handle.init(ab); } diff --git a/plans/todo/security-audit.md b/plans/todo/security-audit.md new file mode 100644 index 00000000..f442ff4c --- /dev/null +++ b/plans/todo/security-audit.md @@ -0,0 +1,1257 @@ +# Security Audit Plan + +## Overview + +A comprehensive security audit of every crypto module in `react-native-quick-crypto`. Each module gets reviewed by a team of specialist sub-agents running in parallel, scanning for vulnerabilities, bad practices, and correctness issues. + +--- + +## Sub-Agent Definitions + +### 1. Crypto Correctness Agent (`crypto-specialist`) + +**Focus:** Algorithm implementation correctness and spec compliance. + +- Verify implementations match relevant standards (NIST FIPS, RFCs, WebCrypto spec) +- Check for proper IV/nonce generation and uniqueness enforcement +- Validate key size constraints and parameter validation +- Confirm AEAD tag lengths and authenticated data handling +- Compare behavior against Node.js `deps/ncrypto` reference +- Verify post-quantum parameter sets (ML-DSA, ML-KEM) match FIPS 203/204 +- Check KDF iteration counts and salt handling (PBKDF2, scrypt, Argon2, HKDF) + +### 2. Memory Safety Agent (`cpp-specialist`) + +**Focus:** C++ memory management, resource leaks, and undefined behavior. + +- Audit all OpenSSL resource handling (EVP_CTX, BIO, BIGNUM, etc.) for proper cleanup +- Verify smart pointer usage — no raw `new`/`delete` or manual `free` +- Check for use-after-free, double-free, and dangling pointer risks +- Validate buffer bounds checking on all native data paths +- Review error paths for resource leaks (early returns, exceptions) +- Check for integer overflow in size calculations +- Verify all `Uint8Array` / `ArrayBuffer` access is bounds-checked + +### 3. Side-Channel & Timing Agent (`crypto-specialist`) + +**Focus:** Timing attacks, side channels, and key material exposure. + +- Verify constant-time comparison (`CRYPTO_memcmp`) for all auth tag checks +- Check for branching on secret data (key bytes, plaintext) +- Audit error messages for key material leakage +- Verify `RAND_bytes` usage (not `rand()` or `Math.random()`) +- Check that key material is zeroed after use where possible +- Review logging/debug output for sensitive data exposure +- Validate `timingSafeEqual` implementation in TypeScript layer + +### 4. TypeScript API Surface Agent (`typescript-specialist`) + +**Focus:** Input validation, type safety, and API misuse prevention. + +- Audit all public API entry points for input validation +- Check for `any` / `unknown` casts that bypass type safety +- Verify Buffer/Uint8Array conversions are safe (offset, length) +- Review error handling — do errors leak internal state? +- Check for prototype pollution vectors in option parsing +- Validate that TypeScript types match actual native behavior +- Review Nitro `.nitro.ts` specs for type mismatches with C++ implementations + +### 5. Dependency & Supply Chain Agent (general-purpose) + +**Focus:** NPM dependency vulnerabilities and supply chain risks. + +- Run `npm audit` / `bun audit` on all workspace packages +- Check for known CVEs in direct and transitive dependencies +- Review `safe-buffer`, `readable-stream`, `events`, `string_decoder` for known issues +- Verify dependency pinning strategy (exact versions vs ranges) +- Check for typosquatting risks in dependency names +- Review `@craftzdog/react-native-buffer` for security patches +- Audit native deps (`blake3`, `ncrypto`, `fastpbkdf2`) for upstream vulnerabilities +- Check CocoaPods (`OpenSSL-Universal`) and Android native deps for known CVEs +- Verify no post-install scripts run arbitrary code +- Review lockfile integrity + +### 6. Build & Distribution Agent (general-purpose) + +**Focus:** Build pipeline security, CI/CD, and artifact integrity. + +- Review GitHub Actions workflows for injection vulnerabilities +- Check for secrets exposure in CI logs +- Verify build reproducibility +- Review Expo plugin (`withRNQC`) for code injection risks +- Check that `.npmignore` / `files` field excludes test fixtures, keys, configs +- Verify no credentials or API keys in committed files + +### 7. Test Coverage Agent (`testing-specialist`) + +**Focus:** Identifying untested code paths, missing edge cases, and gaps in security-relevant test coverage. + +- Compare each module's test suite against its implementation to find untested code paths +- Check for missing negative tests (invalid inputs, malformed data, wrong key sizes) +- Verify edge cases are covered: empty inputs, max-length inputs, zero-length keys +- Confirm error paths are tested (OpenSSL failures, allocation failures, invalid parameters) +- Check for missing cross-algorithm tests (e.g., encrypt with AES-GCM, decrypt with wrong mode) +- Verify test vectors from standards (NIST, RFC, Wycheproof) are used where available +- Identify modules with no tests at all +- Check that AEAD modules test: tag truncation, tag tampering, nonce reuse detection +- Verify KDFs test: minimum iteration/cost enforcement, salt length edge cases +- Check that key exchange modules test: invalid public keys, point-not-on-curve rejection +- Verify post-quantum modules have round-trip and known-answer tests +- Flag any tests that are skipped, commented out, or marked TODO + +--- + +## Module Inventory & Progress + +Each module is audited by all relevant agents. Status key: + +- `[ ]` Not started +- `[~]` In progress +- `[x]` Complete +- `[!]` Issues found — see notes + +### Hashing + +| Module | Crypto | Memory | Timing | API | Tests | Notes | +| ------------------------------ | ------ | ------ | ------ | --- | ----- | ----------------------------------------------- | +| Hash (SHA-1/256/384/512, SHA3) | [!] | [!] | [x] | [!] | [!] | 3H/3M crypto; 3H/3M/1L mem; 4M API; 4H/1M tests | +| HMAC | [!] | [!] | [!] | [!] | [!] | 1H/2M crypto; 2H/3M mem; 2H/4M API; 3H/5M tests | +| KMAC (128/256) | [x] | [!] | [x] | [!] | [!] | 0H/2M crypto; 2H/3M mem; 3H/5M API; 3H/4M tests | +| BLAKE3 | [!] | [!] | [x] | [!] | [!] | 2M crypto; 2H/2M mem; 2H/3M API; 3H/4M tests | + +### Symmetric Encryption + +| Module | Crypto | Memory | Timing | API | Tests | Notes | +| ------------------ | ------ | ------ | ------ | --- | ----- | ---------------------------------------------------------------- | +| AES-CBC | [!] | [!] | [x] | [!] | [!] | 2M crypto; 2H/2M mem; 0 timing; 4H API; 2H/2M/2L tests | +| AES-CTR | [!] | [!] | [x] | [!] | [!] | 2M crypto; 2H/2M mem; 0 timing; 4H API; 2H/1M/1L tests | +| AES-GCM | [!] | [!] | [x] | [!] | [!] | 2M/1L crypto; 1L mem; 0 timing; 4H/2M API; 3H/3M/1L tests | +| AES-CCM | [!] | [!] | [!] | [!] | [!] | 2H/3M crypto; 2H/1M mem; 1H/1M timing; 2H API; 3H/1M tests | +| AES-OCB | [!] | [!] | [x] | [!] | [!] | 2M/1L crypto; 1M mem; 0 timing; 1H/1M API; 2H/2M tests | +| ChaCha20 | [x] | [!] | [!] | [!] | [!] | 1L crypto; 1H/1M mem; 1M timing; 1M/1L API; 2M/1L tests | +| ChaCha20-Poly1305 | [!] | [!] | [!] | [!] | [!] | 1M/1L crypto; 1H/1M mem; 1M timing; 1H/1M API; 1H/3M tests | +| XChaCha20-Poly1305 | [!] | [!] | [!] | [!] | [x] | 2M/1L crypto; 1M mem; 1H/2M timing; 1M API; 1M/1L tests | +| XSalsa20 | [!] | [!] | [!] | [!] | [!] | 1H/2M crypto; 1M/1L mem; 1H timing; 1H/1M API; 2H/1M tests | +| XSalsa20-Poly1305 | [x] | [!] | [!] | [!] | [!] | 1L crypto; 1M mem; 1M timing; 1M API; 1M/1L tests | +| RSA Cipher | [!] | [!] | [!] | [!] | [!] | 2M/1L crypto; 1H/1M mem; 1H/1M timing; 1H/3M API; 1H/3M/1L tests | + +### Key Derivation + +| Module | Crypto | Memory | Timing | API | Tests | Notes | +| --------------- | ------ | ------ | ------ | --- | ----- | ------------------------------------------ | +| PBKDF2 | [!] | [!] | [x] | [!] | [!] | 4H/2M/1L; fastpbkdf2 unchecked returns | +| Scrypt | [!] | [!] | [x] | [!] | [!] | 3H/3M/2L; N power-of-2 not validated | +| HKDF | [!] | [!] | [x] | [!] | [!] | 3H/4M/2L; RFC 5869 max not enforced | +| Argon2 (d/i/id) | [!] | [!] | [x] | [!] | [!] | 3H/4M/2L; no param validation per RFC 9106 | + +### Key Exchange + +| Module | Crypto | Memory | Timing | API | Tests | Notes | +| -------------- | ------ | ------ | ------ | --- | ----- | --------------------------------- | +| Diffie-Hellman | [!] | [!] | [!] | [!] | [!] | 4H/5M/3L; no peer key validation | +| ECDH | [!] | [!] | [!] | [!] | [!] | 3H/4M/3L; no point-on-curve check | + +### Digital Signatures + +| Module | Crypto | Memory | Timing | API | Tests | Notes | +| --------------------- | ------ | ------ | ------ | --- | ----- | ------------------------------------------ | +| Sign/Verify | [!] | [!] | [!] | [!] | [!] | 3H/2M/2L; EVP_PKEY_CTX ownership confusion | +| ECDSA | [!] | [!] | [x] | [!] | [!] | 3H/2M/1L; no curve whitelist for Node API | +| Ed25519/Ed448 | [!] | [!] | [!] | [!] | [!] | 3H/4M/1L; EVP_PKEY leak on import | +| RSA (PKCS1-v1.5, PSS) | [!] | [!] | [x] | [!] | [!] | 1H/4M/1L; min modulus 256 bits | +| DSA | [!] | [x] | [x] | [!] | [!] | 1H/2M/1L; no min modulus enforcement | + +### Post-Quantum + +| Module | Crypto | Memory | Timing | API | Tests | Notes | +| --------------------- | ------ | ------ | ------ | --- | ----- | -------------------------------------- | +| ML-DSA (44/65/87) | [!] | [!] | [x] | [!] | [!] | 2H/5M/3L; double-free risk in signSync | +| ML-KEM (512/768/1024) | [!] | [!] | [!] | [!] | [!] | 2H/5M/2L; shared secret not zeroed | + +### Key Management & Utilities + +| Module | Crypto | Memory | Timing | API | Tests | Notes | +| ------------------- | ------ | ------ | ------ | --- | ----- | ----------------------------------- | +| KeyObjectHandle | [!] | [!] | [x] | [!] | [!] | 2H/3M/2L; 32-byte key misidentified | +| Random | [x] | [!] | [x] | [!] | [!] | 1H/2M/2L; pow(2,31) fragile | +| Prime | [x] | [x] | [x] | [!] | [!] | 0H/2M/1L; no bit size validation | +| Certificate / SPKAC | [x] | [x] | [x] | [x] | [!] | 0H/0M/2L; minimal surface | +| X.509 | [x] | [x] | [x] | [!] | [!] | 0H/2M/2L; no null check on cert\_ | +| WebCrypto Subtle | [!] | N/A | [!] | [!] | [!] | 5H/10M/5L; TS-only API surface | +| Utils / Conversions | [!] | [x] | [!] | [!] | [!] | 1H/2M/1L; timingSafeEqual view bug | + +### Cross-Cutting + +| Area | Agent | Status | Notes | +| ---------------------------------------------- | ---------- | ------ | ------------------------------- | +| NPM dependency audit | Dependency | [ ] | All workspace packages | +| Native dep audit (blake3, ncrypto, fastpbkdf2) | Dependency | [ ] | | +| CocoaPods / Android native deps | Dependency | [ ] | OpenSSL-Universal, libsodium | +| CI/CD pipeline review | Build | [ ] | GitHub Actions | +| Package distribution review | Build | [ ] | .npmignore, published artifacts | +| Expo plugin review | Build | [ ] | withRNQC, sodium integration | + +--- + +## How to Run the Audit + +Launch sub-agents in parallel, grouping by module category. Each agent receives: + +1. The module's C++ files (from `packages/react-native-quick-crypto/cpp//`) +2. The module's TypeScript files (from `packages/react-native-quick-crypto/src/`) +3. The Nitro spec (from `src/specs/.nitro.ts`) +4. Relevant test files (from `example/src/tests/`) +5. This checklist for tracking + +**Example orchestration for a single module (e.g., AES-GCM):** + +``` +Launch in parallel: + 1. crypto-specialist → Read cpp/cipher/GCMCipher.cpp, check correctness & timing + 2. cpp-specialist → Read cpp/cipher/GCMCipher.cpp, check memory safety + 3. typescript-specialist → Read src/cipher.ts + src/specs/cipher.nitro.ts, check API surface + 4. testing-specialist → Read example/src/tests/cipher/, compare against implementation, find gaps +``` + +**For cross-cutting audits:** + +``` +Launch in parallel: + 1. general-purpose (dependency) → Run npm audit, check CVEs, review lockfiles + 2. general-purpose (build) → Review .github/workflows/, expo plugin, .npmignore +``` + +After each module completes, update the status in this table and note any findings. +Move this file to `plans/done/` when the full audit is complete. + +--- + +## Recurring Patterns + +Issues seen across multiple modules. These are systemic and should be addressed project-wide rather than per-module. + +| Pattern | Severity | Modules Affected | Description | +| --------------------------------------------------- | -------- | ------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Raw `new`/`delete` in digest | HIGH | Hash, HMAC, KMAC, BLAKE3 | Raw `new uint8_t[]` without RAII guard; leak window if `make_shared` or intervening code throws. Fix: `std::unique_ptr` with `.release()` into `NativeArrayBuffer`. | +| `double` → integer cast without validation | HIGH | Hash, HMAC, KMAC, BLAKE3 | Length/size parameters arrive as `double` from JS. NaN, Infinity, negative values, and fractions are not validated before `static_cast`. NaN/Infinity casts are UB in C++. | +| `abvToArrayBuffer` ignores byte offset | HIGH | Hash, HMAC, KMAC, BLAKE3 | Shared utility returns `.buffer` without respecting `byteOffset`/`byteLength`. Sliced typed arrays expose the entire backing buffer to native code. Not always on the hot path but exported and dangerous. | +| No digest-once enforcement at TS layer | MEDIUM | Hash, HMAC, KMAC | After `digest()` is called, subsequent `update()`/`digest()` calls should throw. TS layer relies entirely on native to enforce this, producing cryptic errors. BLAKE3 is exempt (non-destructive finalize). | +| Key material retained in TS after native handoff | MEDIUM | HMAC, BLAKE3 | Key stored as instance property after being passed to native. Never read again, never cleared. Increases exposure window via heap dumps/debuggers. | +| OpenSSL error queue not cleared | LOW | Hash, HMAC, KMAC | `ERR_get_error()` pops one error but doesn't clear the queue. Stale errors can pollute subsequent operations. | +| Unsafe `as Encoding` cast in `_transform` | MEDIUM | Hash, HMAC | Stream `_transform` casts `BufferEncoding` → `Encoding` without validation. Unsupported encodings silently misbehave. | +| Stream `_transform`/`_flush` don't propagate errors | MEDIUM | Hash, HMAC | Errors thrown in `update()` crash the process instead of propagating via the stream callback. | +| Test suites lack standard test vectors | HIGH | Hash, BLAKE3 | Hash has no NIST vectors; BLAKE3 has no official keyed_hash/derive_key vectors. Some modes would pass tests even with broken implementations. | +| Subclass destructor leaks EVP_CIPHER_CTX | HIGH | CCMCipher, ChaCha20, ChaCha20-Poly1305 | Destructors set `ctx = nullptr` before parent `~HybridCipher()` runs. Parent sees null and skips `EVP_CIPHER_CTX_free`. Fix: convert `ctx` to `unique_ptr` in base class. | +| `setAAD` ignores Buffer byte offsets | HIGH | AES-GCM, AES-CCM, AES-OCB, ChaCha20-Poly1305 | `setAAD` passes `buffer.buffer` (entire backing ArrayBuffer) ignoring `byteOffset`/`byteLength`. Sliced Buffers send wrong AAD data — direct AEAD integrity violation. | +| No TS-boundary input validation for ciphers | MEDIUM | All symmetric ciphers | Algorithm name, key length, and IV length are not validated at the TypeScript layer. Invalid inputs produce opaque native errors. Node.js validates these early. | +| Stream `_transform`/`_flush` don't propagate errors | MEDIUM | Hash, HMAC, All Ciphers | Errors thrown in `update()`/`final()` crash the process instead of propagating via the stream callback. AEAD auth failures in `_flush` are especially dangerous. | +| `std::memset` for key zeroing may be optimized away | MEDIUM | XChaCha20-Poly1305, XSalsa20-Poly1305 | Non-sodium destructor paths use `std::memset` which compilers may optimize away. Use `OPENSSL_cleanse` or `sodium_memzero` instead. | +| Key material never zeroed in destructor | HIGH | XSalsa20 | `key[32]` and `nonce[24]` arrays persist in freed heap memory. Other libsodium ciphers at least attempt zeroing. | +| RSA error messages enable padding oracles | HIGH | RSA Cipher | Error messages propagate OpenSSL internal strings; `publicDecrypt` has distinguishable error paths (empty buffer vs exception). Combined with PKCS#1 v1.5 support, this risks Bleichenbacher attacks. | +| Prototype pollution in key preparation | HIGH | RSA Cipher | `'key' in key` traverses prototype chain in `preparePublicCipherKey`/`preparePrivateCipherKey`. Polluted `Object.prototype.key` triggers wrong code path. | +| Unbounded data accumulation in libsodium ciphers | MEDIUM | XChaCha20-Poly1305, XSalsa20-Poly1305 | One-shot libsodium API requires buffering all data in `update()`. No size limit; potential OOM and `size_t` overflow on 32-bit platforms. | +| No NIST/RFC test vectors for AEAD ciphers | HIGH | AES-GCM, AES-CCM, AES-OCB | All AEAD modes only use round-trip tests. No authoritative known-answer verification. Consistently-wrong encrypt/decrypt would pass. | +| Zero dedicated tests for CCM and OCB | HIGH | AES-CCM, AES-OCB | Most complex AEAD implementations have no targeted test coverage — only generic round-trip loop. | +| No AEAD API misuse tests | HIGH | AES-GCM, AES-CCM, AES-OCB, ChaCha20-Poly1305 | No tests for: setAAD after update, getAuthTag on decipher, setAuthTag on cipher, missing setAuthTag before decrypt. Common developer mistakes untested. | +| No wrong key/IV size rejection tests | HIGH | All symmetric ciphers | No test verifies that invalid key or IV sizes are properly rejected through the JS layer. | +| Derived key material never zeroed | MEDIUM | PBKDF2, Scrypt, HKDF, Argon2 | Derived key buffers `delete[]`'d without `OPENSSL_cleanse`. Key material persists in freed heap memory. | +| `double` → integer cast without validation (KDFs) | HIGH | PBKDF2, Scrypt, HKDF, Argon2 | All numeric parameters cross JS-to-C++ bridge as `double` and are `static_cast`'d without checking NaN, Infinity, negative, or out-of-range at C++ layer. | +| Raw `new uint8_t[]` allocation (KDFs) | MEDIUM | PBKDF2, Scrypt, HKDF | Output buffers allocated with raw `new`; exception between alloc and `make_shared` leaks. Should use `std::unique_ptr` with release-on-success. | +| Insufficient TS-layer validation (KDFs) | HIGH | Scrypt, HKDF, Argon2 | Scrypt: N power-of-2 unchecked. HKDF: max output length unchecked. Argon2: no param ranges validated. PBKDF2 is only KDF with thorough TS validation. | +| Missing RFC test vectors (KDFs) | HIGH | HKDF, Argon2 | HKDF: 1 of 7 RFC 5869 vectors. Argon2: RFC 9106 vector output not compared against expected value. | +| `keylen=0` handling inconsistent | MEDIUM | PBKDF2, Scrypt, HKDF | TS layers allow `keylen >= 0` but C++ behavior varies: PBKDF2 does `new uint8_t[0]`, Scrypt/HKDF throw. None tested. | +| Missing peer public key validation | HIGH | DH, ECDH | DH: no range check [2, p-2]. ECDH: no point-on-curve validation. Single most critical key exchange gap. | +| Secret material not zeroed (key exchange) | HIGH | DH, ECDH | Shared secrets in `std::vector` not securely erased with `OPENSSL_cleanse` before destruction. | +| Deprecated DH API usage | MEDIUM | DH | Uses deprecated `DH_*` APIs (DH_new, DH_generate_key, DH_set0_pqg) deprecated in OpenSSL 3.x. ECDH correctly uses modern EVP/OSSL_PARAM. | +| Inconsistent minimum key size enforcement | MEDIUM | DH, RSA, DSA | DH: `initWithSize` enforces 2048-bit but `init`/`DhKeyPair` don't. RSA: minimum 256 bits. DSA: minimum 0 bits. | +| Raw `EVP_PKEY*` without smart pointers | MEDIUM | RSA KeyPair, EC KeyPair, Ed KeyPair, Sign/Verify | Raw pointer with manual `EVP_PKEY_free` in destructor. DSA is the only module using `unique_ptr`. Violates C++20 mandate. | +| Private key DER in `std::string` not zeroed | MEDIUM | RSA, EC, DSA, Ed25519 | All modules copy private key DER bytes into `std::string` that is not zeroed before destruction. | +| EVP_PKEY_CTX ownership confusion in DigestSignInit | HIGH | Sign/Verify, Ed25519, ML-DSA | Pre-created `EVP_PKEY_CTX` passed to `EVP_DigestSignInit`; ownership after partial failure is ambiguous, risking double-free. | +| Thread-unsafe `ERR_error_string` | HIGH | Ed25519 | `ERR_error_string(ERR_get_error(), NULL)` uses static internal buffer. Other modules correctly use `ERR_error_string_n`. | +| No key-type validation at C++ layer (signing) | HIGH | Sign/Verify | C++ `sign()`/`verify()` accept any key type. Validation only in TypeScript, bypassable via Nitro bridge. | +| Self-signing tests only (signatures) | MEDIUM | All signature modules | Every test generates keys and verifies own signatures. No standard test vectors; systematic algorithm errors undetectable. | +| No RAII for OpenSSL contexts (PQ) | MEDIUM | ML-DSA, ML-KEM | `EVP_MD_CTX`, `EVP_PKEY_CTX`, BIO objects managed with manual `free` on each error path. | +| Raw `this` in async lambdas | MEDIUM | ML-DSA, ML-KEM, DH | `Promise::async` captures `this` by raw pointer. Object destruction during async execution = use-after-free. | +| `#if !HAS_*` without `#else` | MEDIUM | ML-DSA, ML-KEM | `setVariant` throws on old OpenSSL but falls through to execute unreachable code. | +| Missing NIST KAT vectors (PQ) | MEDIUM | ML-DSA, ML-KEM | All tests are round-trip only. No KAT vectors from FIPS 203/204. | +| Deprecated RSA API in KeyObjectHandle | MEDIUM | KeyObjectHandle | JWK import/export uses `RSA_new()`, `RSA_set0_key()`, `EVP_PKEY_assign_RSA()` — deprecated in OpenSSL 3.x. | +| WebCrypto spec non-compliance | HIGH | Subtle | Algorithm names not case-insensitive. JWK `ext`/`key_ops` not validated. HKDF extractable not enforced. `deriveBits` accepts `deriveKey` usage. | +| `as unknown as` type casts | MEDIUM | Subtle | Multiple `as unknown as SomeType` casts bypass TypeScript type safety with no runtime validation. | + +--- + +## Findings Log + +_Record issues here as they are discovered. Format: `[severity] module — description`_ + +### Hash (SHA-1/256/384/512, SHA3) + +**HIGH:** + +- [HIGH] Hash — `size_t digestSize` compared `< 0` is always false; negative `outputLength` wraps to massive allocation (HybridHash.cpp:98-101) +- [HIGH] Hash — `reinterpret_cast(&hashLength)` aliasing violation; `size_t` is 8 bytes but OpenSSL writes 4 (HybridHash.cpp:111) +- [HIGH] Hash — Raw `new uint8_t[]` without RAII guard; leak window on future code changes (HybridHash.cpp:105) +- [HIGH] Hash — `abvToArrayBuffer` ignores `byteOffset`/`byteLength` on typed array views (utils/conversion.ts:17-25, shared utility) +- [HIGH] Hash — No NIST/RFC standard test vectors for any hash algorithm +- [HIGH] Hash — No double-`digest()` call test (use-after-free prevention path untested) +- [HIGH] Hash — Many algorithms untested via `createHash`: SHA-224, SHA-384, BLAKE2B-512, RIPEMD-160, SHA3-224, etc. +- [HIGH] Hash — `asyncDigest` error paths completely untested (invalid algo, cSHAKE missing length, length % 8) + +**MEDIUM:** + +- [MEDIUM] Hash — `setParams()` deferred to `digest()` instead of `createHash()`; fails late vs Node.js behavior (HybridHash.cpp:94) +- [MEDIUM] Hash — `double` to `uint32_t` truncation for XOF length; NaN/Infinity/fractions pass through (HybridHash.cpp:164) +- [MEDIUM] Hash — Raw pointer members `ctx`/`md` should use `unique_ptr` with custom deleters (HybridHash.hpp:36-37) +- [MEDIUM] Hash — `copy()` shares `md` pointer; dangling if original destroyed first (HybridHash.cpp:141) +- [MEDIUM] Hash — No null check on `buffer->data()` in `update()` (HybridHash.cpp:83) +- [MEDIUM] Hash — `outputLength` validation order: range check before type check, NaN/Infinity pass (hash.ts:51-63) +- [MEDIUM] Hash — `_transform` casts `BufferEncoding` to `Encoding` unsafely (hash.ts:187-194) +- [MEDIUM] Hash — `asyncDigest` doesn't validate `algorithm.name` is a string at runtime (hash.ts:239) +- [MEDIUM] Hash — No `copy()` after `digest()` test, no encoding variant tests, no large input tests + +**LOW:** + +- [LOW] Hash — cSHAKE bits/bytes ambiguity in length parameter (hash.ts:259-273) +- [LOW] Hash — No guard against calling `digest()` twice at TS or C++ layer +- [LOW] Hash — `hashnames.ts` uses `enum` violating project rules +- [LOW] Hash — No unicode hashing tests, no concurrent hash tests, no `outputLength` on non-XOF test + +### HMAC + +**HIGH:** + +- [HIGH] HMAC — No constant-time HMAC verification API; users must manually use `timingSafeEqual` (design gap, matches Node.js) +- [HIGH] HMAC — Raw pointer `EVP_MAC_CTX*` enables double-free on copy, leak on re-init (HybridHmac.hpp:28) +- [HIGH] HMAC — `createHmac()` error path leaves `ctx` allocated but uninitialized; subsequent `update()` = UB (HybridHmac.cpp:36-40) +- [HIGH] HMAC — Key material stored on TS instance after native handoff; never cleared (hmac.ts:17,37) +- [HIGH] HMAC — `abvToArrayBuffer` leaks entire backing buffer (shared utility, not on HMAC path but risky) +- [HIGH] HMAC — `digest()` does not invalidate context; double-`digest()` and `update()`-after-`digest()` = UB (HybridHmac.cpp) +- [HIGH] HMAC — No test for double-`digest()`, `update()`-after-`digest()`, or HMAC verification workflow + +**MEDIUM:** + +- [MEDIUM] HMAC — Empty key handling substitutes `0x00` byte; diverges from spec intent (HybridHmac.cpp:50-55) +- [MEDIUM] HMAC — No digest-once enforcement at TS or C++ layer +- [MEDIUM] HMAC — Raw `new`/`delete` in `digest()` without RAII guard (HybridHmac.cpp:93) +- [MEDIUM] HMAC — `EVP_MD_get_size` return unchecked; -1 wraps to massive `size_t` (HybridHmac.cpp:90) +- [MEDIUM] HMAC — No null check on `ArrayBuffer::data()` in key or update paths (HybridHmac.cpp:47,76) +- [MEDIUM] HMAC — No TS-side guard against double `digest()` (hmac.ts:78-86) +- [MEDIUM] HMAC — Unsafe `as Encoding` cast in `_transform` (hmac.ts:94) +- [MEDIUM] HMAC — No algorithm normalization or validation against known set (hmac.ts:20-22) +- [MEDIUM] HMAC — Stream `_transform`/`_flush` don't propagate errors via callback (hmac.ts:89-99) +- [MEDIUM] HMAC — No tests for SHA3 algorithms, empty `update()`, large data, or `undefined` key + +**LOW:** + +- [LOW] HMAC — `EVP_MAC*` not wrapped in RAII for `createHmac()` scope (HybridHmac.cpp:30-31) +- [LOW] HMAC — OpenSSL error queue not cleared after `ERR_get_error()` calls +- [LOW] HMAC — Dead `algorithm` property stored on TS instance (hmac.ts:16) +- [LOW] HMAC — No prototype pollution guard on `options` passthrough to Transform (hmac.ts:31) + +### KMAC (128/256) + +**HIGH:** + +- [HIGH] KMAC — `abvToArrayBuffer` loses byte offset on sliced views; affects `timingSafeEqual` path in verify (utils/conversion.ts:17-25) +- [HIGH] KMAC — No upper bound on `outputLengthBits`; unbounded allocation in C++ (subtle.ts:752-762, HybridKmac.cpp:71) +- [HIGH] KMAC — Negative `algorithm.length` bypasses zero-length key check in `kmacGenerateKey` (subtle.ts:724-734) +- [HIGH] KMAC — `double` to `size_t` cast without NaN/negative/overflow validation (HybridKmac.cpp:15) +- [HIGH] KMAC — Raw `new uint8_t[]` without RAII guard in `digest()` (HybridKmac.cpp:71) +- [HIGH] KMAC — No KMACXOF (extensible output) tests or documentation of exclusion +- [HIGH] KMAC — Algorithm name passed directly to OpenSSL without C++ validation; no error tests +- [HIGH] KMAC — Empty data input not tested; no NIST vector for 0-byte message + +**MEDIUM:** + +- [MEDIUM] KMAC — Timing leak on signature length mismatch (acceptable, matches Node.js) (subtle.ts:787-789) +- [MEDIUM] KMAC — Raw `new`/`delete` pattern in `digest()` (HybridKmac.cpp:71-80) +- [MEDIUM] KMAC — No null check on `customization` shared_ptr value (HybridKmac.cpp:36) +- [MEDIUM] KMAC — No null check on `key` or `data` shared_ptrs (HybridKmac.cpp:44,61) +- [MEDIUM] KMAC — `exportKeyJWK` return type is `unknown`; cast bypasses type safety (subtle.ts:1477) +- [MEDIUM] KMAC — Unsafe `as JWK` / `as BinaryLike` casts in `kmacImportKey` without validation (subtle.ts:813,841) +- [MEDIUM] KMAC — Streaming interface allows `update` after `digest` without TS guard (specs/kmac.nitro.ts) +- [MEDIUM] KMAC — Zero negative/error tests (8 error paths untested) +- [MEDIUM] KMAC — No tests for non-default output lengths +- [MEDIUM] KMAC — No tests for `generateKey` with custom length or empty vs absent customization + +**LOW:** + +- [LOW] KMAC — No minimum key length enforcement per NIST SP 800-185 (HybridKmac.cpp:47) +- [LOW] KMAC — No `EVP_MAC_final` output length verification (HybridKmac.cpp:73) +- [LOW] KMAC — Single-use digest not documented in TS interface +- [LOW] KMAC — OpenSSL error queue not cleared after `ERR_get_error()` calls +- [LOW] KMAC — No tests for multiple `update()` calls, key size boundaries, or additional NIST vectors + +### BLAKE3 + +**HIGH:** + +- [HIGH] BLAKE3 — Raw `new uint8_t[]` in `digest()` leaks on `make_shared` failure (HybridBlake3.cpp:70) +- [HIGH] BLAKE3 — NaN/Infinity pass length validation; `static_cast(NaN)` is UB (HybridBlake3.cpp:67) +- [HIGH] BLAKE3 — Key material retained in TS `keyData` field after native handoff (blake3.ts:21,38) +- [HIGH] BLAKE3 — `abvToArrayBuffer` byte offset bug (recurring, not on BLAKE3 path but exported) +- [HIGH] BLAKE3 — No official test vectors used for keyed_hash or derive_key modes — zero correctness verification +- [HIGH] BLAKE3 — XOF extended output never verified against known values +- [HIGH] BLAKE3 — keyed_hash mode would pass all tests even with broken implementation + +**MEDIUM:** + +- [MEDIUM] BLAKE3 — Key material not securely erased on destruction; `hasher.key` and stored `key` persist (HybridBlake3.hpp:15) +- [MEDIUM] BLAKE3 — `reset()` silently does nothing if key/context optional is empty (HybridBlake3.cpp:85-94) +- [MEDIUM] BLAKE3 — `memcpy` on `blake3_hasher` struct in `copy()` without `is_trivially_copyable` guard (HybridBlake3.cpp:105) +- [MEDIUM] BLAKE3 — No TS-side validation on digest length parameter (blake3.ts:61-82) +- [MEDIUM] BLAKE3 — `copy()` creates and discards throwaway native object (blake3.ts:90) +- [MEDIUM] BLAKE3 — `key` option typed as `Uint8Array` only, not `BinaryLike` (blake3.ts:13) +- [MEDIUM] BLAKE3 — Output length boundary conditions untested (0, 1, 65535, >65535, negative, fractional) +- [MEDIUM] BLAKE3 — Double-digest, empty-data update, and derive_key reset untested + +**LOW:** + +- [LOW] BLAKE3 — XOF output capped at 65535 bytes; arbitrary undocumented limit (HybridBlake3.cpp:64) +- [LOW] BLAKE3 — Stack-local `keyArray` in `initKeyed()` not zeroed after use (HybridBlake3.cpp:24) +- [LOW] BLAKE3 — `getVersion()` not verified against expected "1.8.2" constant +- [LOW] BLAKE3 — Large input test has no correctness verification (only checks length) + +### AES-CBC + +**HIGH:** + +- [HIGH] AES-CBC — `auth_tag_state` uninitialized in HybridCipher constructor; UB if checked before `setArgs()` (HybridCipher.hpp:62) +- [HIGH] AES-CBC — Integer overflow in `update()`: `in_len + EVP_CIPHER_CTX_block_size(ctx)` overflows `int` near INT_MAX (HybridCipher.cpp:116) +- [HIGH] AES-CBC — `setAAD` passes `buffer.buffer` ignoring `byteOffset`/`byteLength`; sliced Buffer sends wrong AAD (cipher.ts:204-219) +- [HIGH] AES-CBC — Stream `_transform`/`_flush` don't propagate errors via callback; native errors crash process (cipher.ts:182-194) + +**MEDIUM:** + +- [MEDIUM] AES-CBC — No algorithm name validation at TS boundary; invalid names pass to native (cipher.ts:81-123) +- [MEDIUM] AES-CBC — No key length validation; wrong key size produces opaque native error (cipher.ts:81-123) +- [MEDIUM] AES-CBC — No IV length validation; AES-CBC requires 16-byte IV (cipher.ts:81-123) +- [MEDIUM] AES-CBC — `update()` uses raw `new`/`delete`; leak window between alloc and `make_shared` (HybridCipher.cpp:117) +- [MEDIUM] AES-CBC — Intermediate key buffer from JS bridge never zeroed after EVP_CipherInit_ex (HybridCipher.cpp) + +**LOW:** + +- [LOW] AES-CBC — `max_message_size` member uninitialized and unused (HybridCipher.hpp:64) +- [LOW] AES-CBC — OpenSSL error queue not cleared after `ERR_get_error()` calls (HybridCipher.cpp) +- [LOW] AES-CBC — `setAutoPadding` exposed but no TS-side guidance for block-alignment requirement (cipher.ts:196-202) + +### AES-CTR + +**HIGH:** + +- [HIGH] AES-CTR — Same `auth_tag_state` uninitialized UB as AES-CBC (HybridCipher.hpp:62) +- [HIGH] AES-CTR — Same integer overflow in `update()` as AES-CBC (HybridCipher.cpp:116) +- [HIGH] AES-CTR — Same `setAAD` buffer offset bug as AES-CBC (cipher.ts:204-219) +- [HIGH] AES-CTR — Same stream error propagation bug as AES-CBC (cipher.ts:182-194) + +**MEDIUM:** + +- [MEDIUM] AES-CTR — No algorithm/key/IV validation at TS boundary; AES-CTR requires 16-byte IV (cipher.ts:81-123) +- [MEDIUM] AES-CTR — Same raw `new`/`delete` in `update()` as AES-CBC (HybridCipher.cpp:117) + +**LOW:** + +- [LOW] AES-CTR — `setAutoPadding` exposed but meaningless for stream cipher (cipher.ts:196-202) + +### AES-GCM + +**HIGH:** + +- [HIGH] AES-GCM — `setAAD` passes `buffer.buffer` ignoring byte offsets; wrong AAD = AEAD integrity violation (cipher.ts:204-219) +- [HIGH] AES-GCM — `getAuthTag()` callable before `final()` and on decipher instances; may return garbage (cipher.ts:221-223) +- [HIGH] AES-GCM — `authTagLength` options parsing uses `Record` defeating type safety; prototype chain leak (utils/cipher.ts:52) +- [HIGH] AES-GCM — Same stream error propagation bug; GCM auth failure in `_flush` crashes process (cipher.ts:182-194) + +**MEDIUM:** + +- [MEDIUM] AES-GCM — `getAuthTag()` always retrieves 16 bytes from OpenSSL regardless of requested `auth_tag_len` (HybridCipher.cpp:246-260) +- [MEDIUM] AES-GCM — No key size validation (must be 16/24/32) at C++ or TS layer (GCMCipher.cpp) +- [MEDIUM] AES-GCM — No auth tag length validation; GCM allows {4,8,12,13,14,15,16} only (cipher.ts:225-231) +- [MEDIUM] AES-GCM — NIST recommends 12-byte IV; no warning for non-standard IV lengths (cipher.ts) +- [MEDIUM] AES-GCM — `auth_tag_state` uninitialized (inherited from base class) + +**LOW:** + +- [LOW] AES-GCM — Allows zero-length IV; cryptographically disastrous (GCMCipher.cpp:40) + +### AES-CCM + +**HIGH:** + +- [HIGH] AES-CCM — Double key/IV initialization: base `init()` sets key+IV before CCM tag/IV lengths configured (CCMCipher.cpp:9-52) +- [HIGH] AES-CCM — Destructor sets `ctx = nullptr` before parent runs; leaks EVP_CIPHER_CTX every time (CCMCipher.hpp:10-13) +- [HIGH] AES-CCM — `authTagLength` silently defaults to 16 when using general `string` overload; CCM requires explicit tag length (cipher.ts:277-306) +- [HIGH] AES-CCM — `setAAD` does not enforce required `plaintextLength` for CCM; Node.js throws ERR_CRYPTO_INVALID_MESSAGELEN (cipher.ts:204-219) + +**MEDIUM:** + +- [MEDIUM] AES-CCM — `setAAD` skips total plaintext length on decrypt when AAD empty; CCM always requires it (CCMCipher.cpp:180-188) +- [MEDIUM] AES-CCM — `double` to `int` truncation in `setAAD` without NaN/Infinity validation (CCMCipher.cpp:158) +- [MEDIUM] AES-CCM — Tag length validation allows odd values; NIST SP 800-38C requires {4,6,8,10,12,14,16} (HybridCipher.cpp:220-221) +- [MEDIUM] AES-CCM — `kMaxMessageSize` hardcoded for 12-byte nonce only; wrong for other nonce lengths (CCMCipher.hpp:23) +- [MEDIUM] AES-CCM — Tautological `in_len < 0` comparison; `size_t` is unsigned (CCMCipher.cpp:60) +- [MEDIUM] AES-CCM — `setAuthTag` not enforced before decrypt `update()`; decryption without auth proceeds (CCMCipher.cpp:66) + +### AES-OCB + +**HIGH:** + +- [HIGH] AES-OCB — `authTagLength` silently defaults when using general `string` overload; OCB requires explicit tag (cipher.ts:315-319) + +**MEDIUM:** + +- [MEDIUM] AES-OCB — `auth_tag_len` member shadows base class member; inconsistent state between `setArgs()` and OCB methods (OCBCipher.hpp:16) +- [MEDIUM] AES-OCB — `init()` signature hides (not overrides) base class virtual `init()`; polymorphic calls use wrong method (OCBCipher.hpp:10) +- [MEDIUM] AES-OCB — Same `setAAD` buffer offset bug as AES-GCM; integrity violation for AEAD (cipher.ts:204-219) + +**LOW:** + +- [LOW] AES-OCB — Tag length minimum 8 is more restrictive than RFC 7253 (allows 1-16); diverges from Node.js (OCBCipher.cpp:17) + +### ChaCha20 + +**HIGH:** + +- [HIGH] ChaCha20 — Destructor sets `ctx = nullptr` before parent runs; leaks EVP_CIPHER_CTX (ChaCha20Cipher.hpp:10-13) + +**MEDIUM:** + +- [MEDIUM] ChaCha20 — Key/IV validation after context allocation; failed validation leaks `ctx` (ChaCha20Cipher.cpp:43-50) +- [MEDIUM] ChaCha20 — Destructor leaks EVP context retaining key material in freed heap memory (ChaCha20Cipher.hpp:19-22) +- [MEDIUM] ChaCha20 — No IV length validation at TS boundary; requires 16-byte IV (cipher.ts:81-123) + +**LOW:** + +- [LOW] ChaCha20 — `final()` skips `EVP_CipherFinal_ex`; OpenSSL state never properly closed (ChaCha20Cipher.cpp:91) +- [LOW] ChaCha20 — No authentication; by design but callers must be aware + +### ChaCha20-Poly1305 + +**HIGH:** + +- [HIGH] ChaCha20-Poly1305 — Destructor sets `ctx = nullptr` before parent runs; leaks EVP_CIPHER_CTX (ChaCha20Poly1305Cipher.hpp:10-13) +- [HIGH] ChaCha20-Poly1305 — Same `setAAD` buffer offset bug; wrong AAD breaks AEAD authentication (cipher.ts:204-219) + +**MEDIUM:** + +- [MEDIUM] ChaCha20-Poly1305 — `final()` writes to zero-length `new unsigned char[0]` buffer; UB if EVP writes any bytes (ChaCha20Poly1305Cipher.cpp:98-100) +- [MEDIUM] ChaCha20-Poly1305 — Auth tag must be exactly 16 bytes; no TS-side length validation (cipher.ts:225-231) +- [MEDIUM] ChaCha20-Poly1305 — IV must be exactly 12 bytes; no TS-side validation (cipher.ts:81-123) +- [MEDIUM] ChaCha20-Poly1305 — Destructor leaks EVP context retaining key material (ChaCha20Poly1305Cipher.hpp) + +**LOW:** + +- [LOW] ChaCha20-Poly1305 — No AAD-before-update ordering enforcement (ChaCha20Poly1305Cipher.cpp) + +### XChaCha20-Poly1305 + +**HIGH:** + +- [HIGH] XChaCha20-Poly1305 — Non-sodium destructor `std::memset` may be optimized away; key material persists (XChaCha20Poly1305Cipher.cpp:22-26) + +**MEDIUM:** + +- [MEDIUM] XChaCha20-Poly1305 — Unbounded data accumulation in `update()` via `data_buffer_.resize()`; OOM + potential `size_t` overflow on 32-bit (XChaCha20Poly1305Cipher.cpp:59-61) +- [MEDIUM] XChaCha20-Poly1305 — Non-sodium path does not zero `data_buffer_` or `aad_` vectors in destructor (XChaCha20Poly1305Cipher.cpp:27-28) +- [MEDIUM] XChaCha20-Poly1305 — `update()` returns null/empty buffer; streaming interface misleading — all processing in `final()` (XChaCha20Poly1305Cipher.cpp:51-64) +- [MEDIUM] XChaCha20-Poly1305 — No TS-layer guidance that algorithm not in OpenSSL; depends on C++ fallback (cipher.ts) + +**LOW:** + +- [LOW] XChaCha20-Poly1305 — Preprocessor `#ifdef` vs `#if` inconsistency with XSalsa20Cipher.hpp (XChaCha20Poly1305Cipher.hpp:3) +- [LOW] XChaCha20-Poly1305 — Entire message buffered for one-shot libsodium API; large payload DoS risk + +### XSalsa20 + +**HIGH:** + +- [HIGH] XSalsa20 — **CATASTROPHIC**: `crypto_stream_xor` restarts keystream from counter=0 on each `update()` call; identical keystream XORed with different plaintext blocks (XSalsa20Cipher.cpp:44) +- [HIGH] XSalsa20 — Key material (`key[32]`, `nonce[24]`) never zeroed in destructor (XSalsa20Cipher.hpp:19-22) +- [HIGH] XSalsa20 — Zero input validation in TS layer: key size, nonce size, empty data all unchecked (cipher.ts:352-373) + +**MEDIUM:** + +- [MEDIUM] XSalsa20 — Key/nonce validation uses `<` instead of `!=`; accepts oversized keys silently (XSalsa20Cipher.cpp:18,24) +- [MEDIUM] XSalsa20 — `output` and `counter` parameters silently ignored with `@ts-expect-error` (cipher.ts:356-361) + +**LOW:** + +- [LOW] XSalsa20 — `update()` leaks `output` buffer on `crypto_stream_xor` failure (XSalsa20Cipher.cpp:43-47) +- [LOW] XSalsa20 — No authentication; by design but callers must be aware + +### XSalsa20-Poly1305 + +**MEDIUM:** + +- [MEDIUM] XSalsa20-Poly1305 — Non-sodium destructor `std::memset` may be optimized away (XSalsa20Poly1305Cipher.cpp:20-22) +- [MEDIUM] XSalsa20-Poly1305 — Unbounded data accumulation in `update()` with no size limit (XSalsa20Poly1305Cipher.cpp:54-56) +- [MEDIUM] XSalsa20-Poly1305 — No dedicated TS API; behavior through generic `createCipheriv` undefined (cipher.ts) + +**LOW:** + +- [LOW] XSalsa20-Poly1305 — AAD not supported; `setAAD` throws explicit error (correct behavior) (XSalsa20Poly1305Cipher.cpp:104-106) +- [LOW] XSalsa20-Poly1305 — Same buffering design as XChaCha20-Poly1305; large payload concern + +### RSA Cipher + +**HIGH:** + +- [HIGH] RSA Cipher — Raw `EVP_PKEY_CTX*` without RAII; `toOpenSSLPadding()` throw leaks context in 5 methods (HybridRsaCipher.cpp) +- [HIGH] RSA Cipher — RSA PKCS#1 v1.5 padding for decryption enables Bleichenbacher padding oracle; error messages propagate OpenSSL details (HybridRsaCipher.cpp + publicCipher.ts:126,219,248) +- [HIGH] RSA Cipher — Prototype pollution in `preparePublicCipherKey`/`preparePrivateCipherKey`; `'key' in key` traverses prototype chain (publicCipher.ts:86-94,186) + +**MEDIUM:** + +- [MEDIUM] RSA Cipher — `double` to `int` cast for padding param; NaN/Infinity = UB (HybridRsaCipher.cpp:52) +- [MEDIUM] RSA Cipher — `publicDecrypt` returns empty buffer on certain error codes; broad `(err & 0xFF) == 0x04` match masks real failures (HybridRsaCipher.cpp:264) +- [MEDIUM] RSA Cipher — EVP_PKEY_CTX not RAII-wrapped; missed error paths leak context (HybridRsaCipher.cpp) +- [MEDIUM] RSA Cipher — No RSA padding constant validation; `padding: 99` sends invalid value to OpenSSL (publicCipher.ts:113) +- [MEDIUM] RSA Cipher — Error messages may enable padding oracle info leakage (publicCipher.ts:126,148,219,248) +- [MEDIUM] RSA Cipher — Default OAEP hash is SHA-1; deprecated but matches Node.js (publicCipher.ts:114,236) + +**LOW:** + +- [LOW] RSA Cipher — CryptoKey algorithm not validated as RSA before use (publicCipher.ts:63) +- [LOW] RSA Cipher — No RSA key size minimum enforcement at encrypt/decrypt time (HybridRsaCipher.cpp) +- [LOW] RSA Cipher — `RSA_NO_PADDING` not supported but not explicitly rejected with helpful message (HybridRsaCipher.cpp:17-26) + +### Cross-Cutting (Symmetric Encryption) + +**HIGH:** + +- [HIGH] All Ciphers — `abvToArrayBuffer` ignores `byteOffset`/`byteLength`; sliced buffers pass wrong data to native (utils/conversion.ts:17-25) +- [HIGH] All Ciphers — `getUIntOption` uses `Record` defeating type safety for options parsing (utils/cipher.ts:52) +- [HIGH] 3 Subclasses — CCMCipher, ChaCha20Cipher, ChaCha20Poly1305Cipher destructors leak EVP_CIPHER_CTX by nulling `ctx` before parent destructor + +**MEDIUM:** + +- [MEDIUM] All Ciphers — No algorithm name validation at TS boundary; invalid ciphers produce opaque native errors +- [MEDIUM] All Ciphers — `any` casts in stream options filtering (cipher.ts:103-104) +- [MEDIUM] All Ciphers — Double `binaryLikeToArrayBuffer` conversion in Cipheriv/Decipheriv constructors (cipher.ts:248-252) +- [MEDIUM] HybridCipherFactory — `EVP_CIPHER` leaked on exception during `cipherInstance->init()` (HybridCipherFactory.hpp:42-87) +- [MEDIUM] HybridCipherFactory — Switch fallthrough from `EVP_CIPH_STREAM_CIPHER` to `default` without `[[fallthrough]]` (HybridCipherFactory.hpp:80-88) + +**LOW:** + +- [LOW] HybridCipherFactory — `EVP_CIPHER_free(nullptr)` called after failed fetch; no-op but code smell (HybridCipherFactory.hpp:91) +- [LOW] cipher.ts — `authTagLen` defaults to 16 even for non-AEAD ciphers; harmless but unnecessary + +### Test Coverage Gaps (Symmetric Encryption) + +**HIGH:** + +- [HIGH] AES-CBC — No NIST/RFC test vectors with known ciphertext verification; round-trip only +- [HIGH] AES-CBC — No wrong key/IV length rejection tests +- [HIGH] AES-GCM — No NIST SP 800-38D test vectors; correctness unverified against standard +- [HIGH] AES-GCM — No tag truncation / custom tag length tests +- [HIGH] AES-GCM — No wrong key/IV size rejection tests +- [HIGH] AES-CCM — Zero dedicated tests; most complex AEAD has only generic round-trip coverage +- [HIGH] AES-CCM — No tag verification failure test (CCM handles auth in `update`, not `final`) +- [HIGH] AES-CCM — No CCM-specific IV range validation test (7-13 bytes required) +- [HIGH] AES-OCB — Zero dedicated tests; custom init/getAuthTag/setAuthTag all untested +- [HIGH] AES-OCB — No tag tampering or authentication failure test +- [HIGH] XSalsa20 — Single round-trip test with no vectors, no key/nonce size rejection tests +- [HIGH] XSalsa20 — Oversized key silently accepted due to `<` vs `!=` in C++ validation +- [HIGH] RSA — No cross-padding-mode failure test (OAEP encrypt → PKCS1 decrypt should fail) +- [HIGH] All AEAD — No test for `setAAD` after `update` (must error) +- [HIGH] All AEAD — No test for `getAuthTag` on decipher (must error) +- [HIGH] All AEAD — No test for `setAuthTag` on cipher (must error) + +**MEDIUM:** + +- [MEDIUM] AES-CBC — No empty plaintext test (exercises padding-only output) +- [MEDIUM] AES-CBC — No `setAutoPadding` test +- [MEDIUM] AES-GCM — No tampered AAD test (modified AAD should cause auth failure) +- [MEDIUM] AES-GCM — No test for missing `setAuthTag` on decrypt +- [MEDIUM] AES-GCM — No test for `getAuthTag` before `final()` +- [MEDIUM] AES-CCM — No custom tag length test (4-16 even values) +- [MEDIUM] AES-OCB — No custom tag length test (8-16 bytes) +- [MEDIUM] AES-OCB — No wrong tag length rejection test +- [MEDIUM] ChaCha20 — No wrong key size rejection test (32 bytes required) +- [MEDIUM] ChaCha20 — RFC vectors only cover encryption direction; no standalone decryption test +- [MEDIUM] ChaCha20-Poly1305 — No wrong key size rejection test +- [MEDIUM] ChaCha20-Poly1305 — No tag tampering test +- [MEDIUM] ChaCha20-Poly1305 — No tampered ciphertext test +- [MEDIUM] ChaCha20-Poly1305 — Custom tag length tests may be silently wrong; C++ always uses 16 bytes +- [MEDIUM] XChaCha20-Poly1305 — No tampered ciphertext test (only wrong tag tested) +- [MEDIUM] XSalsa20 — Uses standalone `xsalsa20()` not `createCipheriv`; path untested +- [MEDIUM] XSalsa20-Poly1305 — "Test vector" only does round-trip; no known-answer comparison +- [MEDIUM] RSA — No unsupported padding constant rejection test +- [MEDIUM] RSA — No malformed ciphertext test for `privateDecrypt` +- [MEDIUM] RSA — No wrong key type test (`publicEncrypt` with private key behavior undefined) +- [MEDIUM] All modules — No key/IV type validation tests (null, undefined, number as key) +- [MEDIUM] All modules — Generic round-trip error catch converts all errors to `expect.fail`, masking root cause + +**LOW:** + +- [LOW] AES-CBC — No stream interface tests (pipe/transform API) +- [LOW] AES-CTR/CFB/OFB/ECB — No mode-specific edge case tests +- [LOW] AES-GCM — No empty AAD test +- [LOW] ChaCha20 — Silently catches 64-bit nonce failure with bare `catch {}` (chacha_tests.ts:310-313) +- [LOW] XChaCha20-Poly1305 — Only one IETF test vector; additional Wycheproof vectors would strengthen confidence +- [LOW] XSalsa20-Poly1305 — No tampered ciphertext test (only tag mismatch tested) +- [LOW] RSA — No minimum key size test (1024-bit) +- [LOW] All modules — No multi-chunk `update()` tests (all tests use single update call) + +### PBKDF2 + +**HIGH:** + +- [HIGH] PBKDF2 — No return value check on `PKCS5_PBKDF2_HMAC`; failed derivation silently returns uninitialized buffer (HybridPbkdf2.cpp:44) +- [HIGH] PBKDF2 — No return value check on `fastpbkdf2_hmac_*` calls; failure returns uninitialized heap data as derived key material (HybridPbkdf2.cpp:27-34) +- [HIGH] PBKDF2 — Raw `new uint8_t[]` without RAII guard; leak if `make_shared` throws (HybridPbkdf2.cpp:22-23) +- [HIGH] PBKDF2 — `keylen=0` not rejected at C++ layer; `new uint8_t[0]` is implementation-defined (HybridPbkdf2.cpp:21-22) + +**MEDIUM:** + +- [MEDIUM] PBKDF2 — `double` to `uint32_t` truncation for iterations unchecked at C++ layer (HybridPbkdf2.cpp:28) +- [MEDIUM] PBKDF2 — Derived key material never zeroed; `delete[]` without `OPENSSL_cleanse` (HybridPbkdf2.cpp:23) +- [MEDIUM] PBKDF2 — `password.get()->data()` dereference without null check; null ArrayBuffer = UB (HybridPbkdf2.cpp:27) +- [MEDIUM] PBKDF2 — No digest validation at C++ layer; unknown digests silently fall through to OpenSSL path (HybridPbkdf2.cpp:26-45) + +**LOW:** + +- [LOW] PBKDF2 — `reinterpret_cast` discards const; should be `const char*` (HybridPbkdf2.cpp:41) +- [LOW] PBKDF2 — Async test assertions inside callback are fire-and-forget; failures silently swallowed (pbkdf2_tests.ts:30-35) + +### Scrypt + +**HIGH:** + +- [HIGH] Scrypt — No validation of `N` being a power of 2 per RFC 7914; OpenSSL rejects with opaque error (scrypt.ts:38-45) +- [HIGH] Scrypt — No upper-bound validation of N, r, p; `N=2^30, r=8, p=1` requires ~1TB memory (scrypt.ts:38-45) +- [HIGH] Scrypt — `keylen=0` passes TS validation but C++ throws; inconsistent with Node.js which returns empty Buffer (scrypt.ts:88, HybridScrypt.cpp:36-38) + +**MEDIUM:** + +- [MEDIUM] Scrypt — Derived key material never zeroed before deallocation (HybridScrypt.cpp:59) +- [MEDIUM] Scrypt — `double` to `uint64_t` truncation unchecked; negative doubles wrap to large positive values (HybridScrypt.cpp:30-34) +- [MEDIUM] Scrypt — No `keylen` upper-bound validation; massive allocation possible (scrypt.ts:88,119) +- [MEDIUM] Scrypt — `maxmem` default of 32MB may OOM mobile devices; no documentation (scrypt.ts:35) + +**LOW:** + +- [LOW] Scrypt — Missing `Number.isInteger(keylen)` check; float values truncated silently (scrypt.ts:88) +- [LOW] Scrypt — Error in async path casts to `Error` with `err as Error`; OpenSSL errors may be strings (scrypt.ts:103) + +### HKDF + +**HIGH:** + +- [HIGH] HKDF — No maximum output length validation per RFC 5869; must not exceed `255 * HashLen` (hkdf.ts:64, HybridHkdf.cpp:77-81) +- [HIGH] HKDF — Passing `nullptr` to `OSSL_PARAM_construct_octet_string` for empty key; may segfault (HybridHkdf.cpp:57) +- [HIGH] HKDF — `OSSL_PARAM params[5]` array exactly full; any additional param would overflow (HybridHkdf.cpp:44) + +**MEDIUM:** + +- [MEDIUM] HKDF — `keylen=0` allowed by TS but rejected by C++; inconsistent with Node.js (hkdf.ts:64) +- [MEDIUM] HKDF — Derived key material not zeroed before deallocation (HybridHkdf.cpp:93) +- [MEDIUM] HKDF — `hkdfDeriveBits` returns ArrayBuffer directly; `Math.ceil(length / 8)` excess bytes not trimmed (hkdf.ts:132,146) +- [MEDIUM] HKDF — Error reporting uses raw `ERR_get_error()` numeric code, not human-readable string (HybridHkdf.cpp:34,40,88) +- [MEDIUM] HKDF — Salt silently omitted when empty; RFC 5869 defaults to `HashLen` zeros; behavior undocumented (HybridHkdf.cpp:60-66) + +**LOW:** + +- [LOW] HKDF — `keylen` not validated as integer; float values silently truncated (hkdf.ts:64) +- [LOW] HKDF — Empty info buffer silently omitted; RFC 5869 defaults to empty string (HybridHkdf.cpp:70-72) + +### Argon2 + +**HIGH:** + +- [HIGH] Argon2 — No RFC 9106 parameter validation: min salt 8 bytes, min tag 4 bytes, min memory `8*parallelism` KiB, min passes 1, min parallelism 1 (argon2.ts:43-58, HybridArgon2.cpp:26-60) +- [HIGH] Argon2 — `double` to `uint32_t` truncation for parallelism/memory/passes/version; NaN/Infinity/negative = UB (HybridArgon2.cpp:50) +- [HIGH] Argon2 — `tagLength` cast from `double` to `size_t` can produce huge allocations (HybridArgon2.cpp:50) + +**MEDIUM:** + +- [MEDIUM] Argon2 — No version validation; only `0x10` and `0x13` are defined per RFC 9106 (argon2.ts:45, HybridArgon2.cpp:50) +- [MEDIUM] Argon2 — Derived key material not zeroed; neither ncrypto buffer nor copy are cleansed (HybridArgon2.cpp:59) +- [MEDIUM] Argon2 — `hashSync` passes original shared_ptrs without copying; potential use-after-free edge case (HybridArgon2.cpp:97) +- [MEDIUM] Argon2 — Error message may expose OpenSSL internal reason strings (HybridArgon2.cpp:54-56) + +**LOW:** + +- [LOW] Argon2 — Algorithm name string echoed in error message (HybridArgon2.cpp:23) +- [LOW] Argon2 — `argon2d` exposed without warning; vulnerable to side-channel attacks (argon2.ts:29-37) + +### Test Coverage Gaps (Key Derivation) + +**HIGH:** + +- [HIGH] PBKDF2 — Async test assertions inside callbacks are fire-and-forget; failures silently swallowed (pbkdf2_tests.ts:30-35) +- [HIGH] Scrypt — No negative/error-path tests: invalid N (not power of 2), N=0, r=0, p=0, negative params (scrypt_tests.ts) +- [HIGH] HKDF — Only 1 of 7 RFC 5869 test vectors implemented; missing zero-length salt/info case (hkdf_tests.ts:8-17) +- [HIGH] HKDF — No tests for empty salt, empty info, or empty key; critical RFC 5869 edge cases (hkdf_tests.ts) +- [HIGH] Argon2 — RFC 9106 test vector output not verified; only checks `result.length === 32`, not actual bytes (argon2_tests.ts:23-27) +- [HIGH] Argon2 — No error-path tests for invalid parameters: zero parallelism, zero memory, short salt, NaN (argon2_tests.ts) + +**MEDIUM:** + +- [MEDIUM] PBKDF2 — No test for `keylen=0`; Node.js returns empty Buffer (pbkdf2_tests.ts) +- [MEDIUM] HKDF — No negative/error-path tests: invalid digest, negative keylen, keylen exceeding 255\*HashLen (hkdf_tests.ts) +- [MEDIUM] Scrypt — Missing RFC 7914 Test Case 4 (N=1048576, r=8, p=1); should be documented (scrypt_tests.ts:12-43) +- [MEDIUM] All KDFs — No cross-validation tests with Node.js `crypto` module output + +**LOW:** + +- [LOW] PBKDF2 — Test at line 100-106 labeled "should throw if password not string/ArrayBuffer" actually tests "No callback provided" (pbkdf2_tests.ts:97-106) + +### Diffie-Hellman + +**HIGH:** + +- [HIGH] DH — No minimum prime size enforcement when custom prime provided via `init()`; accepts dangerously small primes (HybridDiffieHellman.cpp:25) +- [HIGH] DH — No validation of peer public key in `computeSecret()`; 0, 1, or p-1 enable small subgroup attacks (HybridDiffieHellman.cpp:129-213) +- [HIGH] DH — Shared secret in `std::vector` not zeroed on destruction; persists in heap (HybridDiffieHellman.cpp:204) +- [HIGH] DhKeyPair — No minimum prime size enforcement in `generateKeyPairSync()` unlike `HybridDiffieHellman::initWithSize()` (HybridDhKeyPair.cpp:85-107) + +**MEDIUM:** + +- [MEDIUM] DH — `double` to `int` cast for `primeLength` and `generator` without range validation; NaN/Infinity = UB (HybridDhKeyPair.cpp:27-28,35-36) +- [MEDIUM] DH — Private key material not zeroed on destruction; BIGNUM temporaries freed with `BN_free` not `BN_clear_free` (HybridDiffieHellman.cpp:355-425) +- [MEDIUM] DH — No generator validation; generator 0 or 1 produces degenerate keys (HybridDiffieHellman.cpp:25-64) +- [MEDIUM] DH — Missing named groups; only modp14-18 defined; no groups with known subgroup order (dh-groups.ts) +- [MEDIUM] DH — `createDiffieHellman` default encoding 'utf8' inconsistent with Node.js 'binary' (diffie-hellman.ts:156) + +**LOW:** + +- [LOW] DH — BIO resource management uses manual `BIO_free` instead of RAII; leak on exception (HybridDhKeyPair.cpp:137-175) +- [LOW] DH — `generateKeyPair()` captures `this` in async lambda; use-after-free if object destroyed (HybridDhKeyPair.cpp:40) +- [LOW] DH — `ToNativeArrayBuffer` uses raw `new uint8_t[]` with lambda deleter (QuickCryptoUtils.hpp:38-41) + +### ECDH + +**HIGH:** + +- [HIGH] ECDH — No explicit point-on-curve validation for peer public key in `computeSecret()`; invalid-curve attack possible (HybridECDH.cpp:62-100) +- [HIGH] ECDH — No validation of private key range [1, n-1] in `setPrivateKey()`; key of 0 produces point at infinity (HybridECDH.cpp:120-149) +- [HIGH] ECDH — Shared secret `std::vector` not securely erased before destruction (HybridECDH.cpp:92-99) + +**MEDIUM:** + +- [MEDIUM] ECDH — `setPublicKey()` does not validate point is on configured curve (HybridECDH.cpp:169-174) +- [MEDIUM] ECDH — No curve restriction/allowlist; weak/deprecated curves accepted via `OBJ_txt2nid` (HybridECDH.cpp:217-226) +- [MEDIUM] ECDH — `double format` cast to `point_conversion_form_t` without validation at C++ layer (HybridECDH.cpp:176-209) +- [MEDIUM] ECDH — Private key bytes from `getPrivateKey()` not padded to curve field size; interop issues (HybridECDH.cpp:102-118) +- [MEDIUM] ECDH — `createEcEvpPkey()` does not check return values of `OSSL_PARAM_BLD_push_*` calls (QuickCryptoUtils.cpp:15-18) + +**LOW:** + +- [LOW] ECDH — `_curveNid` initialized to 0 (`NID_undef`); magic number dependency (HybridECDH.hpp:37) +- [LOW] ECDH — Static singleton `_convertKeyHybrid` lazily created, never destroyed (ecdh.ts:11-18) +- [LOW] ECDH — `getPublicKey()` ignores `format` parameter; no compressed/hybrid format support (ecdh.ts:69) + +### Test Coverage Gaps (Key Exchange) + +**HIGH:** + +- [HIGH] DH — No invalid public key test; 0, 1, or p-1 (small subgroup attack vectors) not tested +- [HIGH] ECDH — No invalid public key test; point not on curve and identity point untested +- [HIGH] ECDH — No private key range test; value 0 or >= curve order n untested + +**MEDIUM:** + +- [MEDIUM] DH — No weak prime test; non-safe prime detection via `verifyError` untested +- [MEDIUM] ECDH — No cross-curve test; Alice P-256 / Bob P-384 `computeSecret` behavior untested +- [MEDIUM] DH/ECDH — No empty input test for `computeSecret`, `setPublicKey`, `setPrivateKey` +- [MEDIUM] DH — No NIST/RFC known-answer tests; only checks Alice == Bob +- [MEDIUM] ECDH — No NIST CAVP or RFC 5903 test vectors; only checks Alice == Bob +- [MEDIUM] DhKeyPair — `DhKeyPairGen` class has no dedicated tests at all + +**LOW:** + +- [LOW] ECDH — No weak curve test (e.g., prime192v1) +- [LOW] DH — No string encoding roundtrip test through `computeSecret` + +### Sign/Verify (Generic Interface) + +**HIGH:** + +- [HIGH] Sign/Verify — Raw `EVP_PKEY*` via `GetAsymmetricKey().get()` with no reference-count guarantee during async operations (HybridSignHandle.cpp:127, HybridVerifyHandle.cpp:126) +- [HIGH] Sign/Verify — No validation that key type matches operation; C++ accepts any key (HybridSignHandle.cpp:119-131, HybridVerifyHandle.cpp:119-130) +- [HIGH] Sign/Verify — `EVP_PKEY_CTX` double-free risk on partial `EVP_DigestSignInit` failure (HybridSignHandle.cpp:150-165) + +**MEDIUM:** + +- [MEDIUM] Sign/Verify — Unbounded `data_buffer` accumulation in streaming mode; DoS vector (HybridSignHandle.hpp:33, HybridVerifyHandle.hpp:33) +- [MEDIUM] Sign/Verify — No re-use protection; calling `sign()`/`verify()` twice on finalized context = UB (HybridSignHandle.cpp:194) + +**LOW:** + +- [LOW] Sign/Verify — Error message leaks key type info and internal constants (HybridSignHandle.cpp:206-208) +- [LOW] Sign/Verify — `size_t` to `int` narrowing in `BN_bn2binpad` (SignUtils.hpp:50,64) + +### ECDSA (EC Key Pairs) + +**HIGH:** + +- [HIGH] ECDSA — No curve validation for Node.js API key generation; arbitrary curve names accepted including weak curves (ec.ts:382-396, HybridEcKeyPair.cpp:317) +- [HIGH] ECDSA — Raw `EVP_PKEY*` without RAII; violates project smart-pointer mandate (HybridEcKeyPair.hpp:44) +- [HIGH] ECDSA — BIO not checked for null before `i2d_PKCS8PrivateKey_bio`; null = UB (HybridEcKeyPair.cpp:286) + +**MEDIUM:** + +- [MEDIUM] ECDSA — SHA-1 allowed for ECDSA signing; WebCrypto spec recommends rejection (HybridEcKeyPair.cpp:343) +- [MEDIUM] ECDSA — Private key DER data in `std::string` not zeroed before destruction (HybridEcKeyPair.cpp:210-211,290-291) +- [MEDIUM] ECDSA — `importKey` tries multiple parsing strategies without clearing error queue between attempts (HybridEcKeyPair.cpp:131-166) + +**LOW:** + +- [LOW] ECDSA — Signature malleability (high-S) not enforced; DER-encoded ECDSA signatures malleable by design (SignUtils.hpp:42-53) + +### Ed25519/Ed448 + +**HIGH:** + +- [HIGH] Ed25519/Ed448 — Memory leak for imported keys; `EVP_PKEY*` created in `importPublicKey`/`importPrivateKey` never freed by callers (HybridEdKeyPair.cpp:356-401, callers at :155,:221) +- [HIGH] Ed25519/Ed448 — Double-free risk with `EVP_PKEY_CTX` in `signSync` on partial `EVP_DigestSignInit` failure (HybridEdKeyPair.cpp:163-173) +- [HIGH] Ed25519/Ed448 — `ERR_error_string(ERR_get_error(), NULL)` uses thread-unsafe static buffer (HybridEdKeyPair.cpp:172,241) + +**MEDIUM:** + +- [MEDIUM] Ed25519/Ed448 — Raw `new`/`delete` throughout; violates C++20 smart-pointer mandate (HybridEdKeyPair.cpp:58,181,282-284,295-296,339-340) +- [MEDIUM] Ed25519/Ed448 — Private key raw bytes not zeroed; `delete[]` without `OPENSSL_cleanse` (HybridEdKeyPair.cpp:339-343) +- [MEDIUM] Ed25519/Ed448 — Incomplete curve name normalization; `"ED25519"` uppercase not handled (HybridEdKeyPair.cpp:359-366) +- [MEDIUM] Ed25519/Ed448 — `cipher`/`passphrase` parameters accepted but ignored; encrypted export silently doesn't encrypt (HybridEdKeyPair.cpp:84-86) + +**LOW:** + +- [LOW] Ed25519/Ed448 — No validation of raw key sizes before `EVP_PKEY_new_raw_public_key`; generic error (HybridEdKeyPair.cpp:369) + +### RSA (PKCS1-v1.5, PSS) Key Generation + +**HIGH:** + +- [HIGH] RSA KeyGen — Minimum modulus length 256 bits is trivially factorable; NIST minimum is 2048 (rsa.ts:73,201) + +**MEDIUM:** + +- [MEDIUM] RSA KeyGen — Raw `EVP_PKEY*` without RAII; violates smart-pointer mandate (HybridRsaKeyPair.hpp:35) +- [MEDIUM] RSA KeyGen — Private key DER data in `std::string` not zeroed (HybridRsaKeyPair.cpp:132) +- [MEDIUM] RSA KeyGen — Public exponent not validated; exponents of 1 or even numbers produce degenerate keys (HybridRsaKeyPair.cpp:57) +- [MEDIUM] RSA KeyGen — `hashAlgorithm` stored but unused in key generation; RSA-PSS keys lack restricted parameters (HybridRsaKeyPair.cpp:86-88) + +**LOW:** + +- [LOW] RSA KeyGen — `double` to `int` truncation for `modulusLength` unchecked (HybridRsaKeyPair.cpp:76) + +### DSA + +**HIGH:** + +- [HIGH] DSA — No minimum modulus length validation; `modulusLength=512` accepted; FIPS 186-4 requires 1024+ (HybridDsaKeyPair.cpp:29, dsa.ts:37) + +**MEDIUM:** + +- [MEDIUM] DSA — No validation of `divisorLength` against FIPS 186-4 (L,N) pairs: (1024,160), (2048,224), (2048,256), (3072,256) (HybridDsaKeyPair.cpp:50-53) +- [MEDIUM] DSA — DSA deprecated in FIPS 186-5 (2023); no warning or deprecation notice to users + +**LOW:** + +- [LOW] DSA — Private key DER data in `std::string` not zeroed (HybridDsaKeyPair.cpp:121-122) + +### Test Coverage Gaps (Digital Signatures) + +**HIGH:** + +- [HIGH] All Signatures — No NIST/RFC test vector validation; all tests use self-generated keys/signatures +- [HIGH] All Signatures — No cross-implementation verification (Node.js crypto → RNQC or vice versa) +- [HIGH] ECDSA — No signature malleability tests (high-S value handling) +- [HIGH] Ed25519/Ed448 — No tests with wrong-type key (e.g., X25519 key for Ed25519 sign) +- [HIGH] Ed448 — No signing tests at all; only key generation and export tested +- [HIGH] RSA — No key size boundary tests; 512-bit or 1024-bit key generation not tested for rejection + +**MEDIUM:** + +- [MEDIUM] DSA — No tests for `dsaEncoding: 'ieee-p1363'` format +- [MEDIUM] All — No empty-message signing tests +- [MEDIUM] RSA-PSS — No salt length edge case tests (`RSA_PSS_SALTLEN_DIGEST`, `RSA_PSS_SALTLEN_MAX_SIGN`) +- [MEDIUM] All — No concurrent/parallel signing tests (thread-safety of `ERR_error_string` static buffer) + +### ML-DSA (44/65/87) + +**HIGH:** + +- [HIGH] ML-DSA — Double-free risk on `EVP_PKEY_CTX` in `signSync`/`verifySync`; ownership ambiguous after partial `EVP_DigestSignInit` failure (HybridMlDsaKeyPair.cpp:180-182,231-233) +- [HIGH] ML-DSA — Unnecessary `EVP_PKEY_CTX_new_from_name` before `EVP_DigestSignInit`; pre-created context with wrong algorithm = UB (HybridMlDsaKeyPair.cpp:174,225) + +**MEDIUM:** + +- [MEDIUM] ML-DSA — No RAII for `EVP_MD_CTX` and manual `new[]`/`delete[]` for signature buffers (HybridMlDsaKeyPair.cpp:169,192) +- [MEDIUM] ML-DSA — Raw `this` capture in `Promise::async` lambdas; use-after-free if object destroyed (HybridMlDsaKeyPair.cpp:42,159,210) +- [MEDIUM] ML-DSA — No FIPS 204 context string support; applications cannot use domain separation (HybridMlDsaKeyPair.cpp:162-203) +- [MEDIUM] ML-DSA — `setVariant` lacks `#else` guard; throw doesn't prevent compilation of unreachable code (HybridMlDsaKeyPair.cpp:31-33) +- [MEDIUM] ML-DSA — `double` parameters for format/type enums cast to `int` without range/NaN checking (HybridMlDsaKeyPair.cpp:57-60) + +**LOW:** + +- [LOW] ML-DSA — `getEvpPkeyType()` defined but never called; dead code (HybridMlDsaKeyPair.cpp:18-28) +- [LOW] ML-DSA — Private key export always unencrypted; no cipher/passphrase support (HybridMlDsaKeyPair.cpp:134) +- [LOW] ML-DSA — Private key bytes in BIO buffers not zeroed before `delete[]` (HybridMlDsaKeyPair.cpp:145-153) + +### ML-KEM (512/768/1024) + +**HIGH:** + +- [HIGH] ML-KEM — Shared secret not zeroed after use in `encapsulateSync`; raw `sharedKey` ArrayBuffer may be long-lived (HybridMlKemKeyPair.cpp:246-264, subtle.ts:2778) +- [HIGH] ML-KEM — No key type validation in `_encapsulateCore`/`_decapsulateCore`; no check that key is public/private (subtle.ts:2752-2809) + +**MEDIUM:** + +- [MEDIUM] ML-KEM — No RAII for `EVP_PKEY_CTX` in encapsulate/decapsulate (HybridMlKemKeyPair.cpp:226-264,280-311) +- [MEDIUM] ML-KEM — Raw `this` capture in `Promise::async` lambdas (HybridMlKemKeyPair.cpp:29,216,270) +- [MEDIUM] ML-KEM — `BIO_new_mem_buf` casts `size_t` to `int` for key data size (HybridMlKemKeyPair.cpp:158,191) +- [MEDIUM] ML-KEM — `setPublicKey` overwrites entire `pkey_` including private key; subsequent `decapsulate` fails confusingly (HybridMlKemKeyPair.cpp:176) +- [MEDIUM] ML-KEM — Packed encapsulation result uses native byte order `memcpy` for `uint32_t` but TS unpacks as little-endian; breaks on big-endian (HybridMlKemKeyPair.cpp:250-251, mlkem.ts:51) + +**LOW:** + +- [LOW] ML-KEM — No validation that imported key matches configured variant (HybridMlKemKeyPair.cpp:145-178) +- [LOW] ML-KEM — Decapsulated shared secret not zeroed before `delete[]` (HybridMlKemKeyPair.cpp:299-309) + +### Test Coverage Gaps (Post-Quantum) + +**HIGH:** + +- [HIGH] ML-DSA/ML-KEM — No NIST Known-Answer Tests (KAT); all tests are round-trip only +- [HIGH] ML-DSA/ML-KEM — No cross-parameter-set rejection tests (e.g., ML-DSA-44 key vs ML-DSA-65 signature) +- [HIGH] ML-KEM — No invalid ciphertext tests; FIPS 203 implicit rejection behavior untested + +**MEDIUM:** + +- [MEDIUM] ML-DSA — No context string tests (API does not support context strings) +- [MEDIUM] ML-KEM — No `encapsulateKey`/`decapsulateKey` end-to-end test using derived AES key for actual encryption +- [MEDIUM] ML-DSA/ML-KEM — No non-extractable key export rejection tests + +### KeyObjectHandle + +**HIGH:** + +- [HIGH] KeyObjectHandle — 32-byte raw key unconditionally assumed X25519; Ed25519 keys silently misidentified (HybridKeyObjectHandle.cpp:757-759) +- [HIGH] KeyObjectHandle — Private key export has no access control; no mechanism to mark keys non-exportable (HybridKeyObjectHandle.cpp:90-216) + +**MEDIUM:** + +- [MEDIUM] KeyObjectHandle — Deprecated RSA API usage: `EVP_PKEY_get0_RSA()`, `RSA_new()`, `RSA_set0_key()` etc. (HybridKeyObjectHandle.cpp:241-248,507-509) +- [MEDIUM] KeyObjectHandle — BIGNUM memory leak in JWK EC import error path; partial null results leak decoded BIGNUMs (HybridKeyObjectHandle.cpp:547-553) +- [MEDIUM] KeyObjectHandle — `base64url_to_bn` returns raw `BIGNUM*` without ownership semantics; error-prone (HybridKeyObjectHandle.cpp:51) +- [MEDIUM] KeyObjectHandle — OpenSSL error details leaked in error messages (KeyObjectData.cpp:49-53,143-146,169-171) + +**LOW:** + +- [LOW] KeyObjectHandle — `setKeyObjectData` is public with no validation (HybridKeyObjectHandle.hpp:48-49) +- [LOW] KeyObjectHandle — `keyDetail` returns empty struct for non-RSA/non-EC keys (HybridKeyObjectHandle.cpp:709-749) + +### Random (CSPRNG) + +**HIGH:** + +- [HIGH] Random — `pow(2, 31) - 1` floating-point comparison for max size; should use `INT32_MAX` (HybridRandom.cpp:12,42) + +**MEDIUM:** + +- [MEDIUM] Random — `abvToArrayBuffer` returns full backing buffer; native layer receives larger-than-expected buffer (conversion.ts:17-25, random.ts:80-88) +- [MEDIUM] Random — Debug `printData` function left in header; prints raw byte data to stdout (HybridRandom.hpp:24-31) + +**LOW:** + +- [LOW] Random — `checkSize` uses floating-point `pow` for integer comparison (HybridRandom.cpp:12) +- [LOW] Random — `CheckIsUint32(1.5)` returns true; non-integer values not rejected (QuickCryptoUtils.hpp:63-65) + +### Prime + +**MEDIUM:** + +- [MEDIUM] Prime — No validation of bit size parameter; 0, negative, or huge values passed to `BN_generate_prime_ex2` (HybridPrime.cpp:18, prime.ts) +- [MEDIUM] Prime — `checkPrime` default `checks=0` meaning ("use OpenSSL default") not documented (prime.ts:105) + +**LOW:** + +- [LOW] Prime — `rem` without `add` silently ignored; matches Node.js but confusing (HybridPrime.cpp:16-35) + +### Certificate / SPKAC + +**LOW:** + +- [LOW] Certificate — No size validation on SPKAC input; extremely large input = excessive allocation (HybridCertificate.cpp:8-40) +- [LOW] Certificate — `OPENSSL_free(buf.data)` leaks if `ToNativeArrayBuffer` throws (HybridCertificate.cpp:29-39) + +### X.509 + +**MEDIUM:** + +- [MEDIUM] X.509 — No null check on `cert_` member before method calls; crash if `init()` never called (HybridX509Certificate.cpp:27-95) +- [MEDIUM] X.509 — `validFromDate`/`validToDate` cast `time_t` to `double` × 1000; precision safe for practical dates (HybridX509Certificate.cpp:51-57) + +**LOW:** + +- [LOW] X.509 — `fingerprint()` uses SHA-1; provided for compatibility, `fingerprint256`/`fingerprint512` also available (HybridX509Certificate.cpp:78) +- [LOW] X.509 — TypeScript caches immutable properties but never clears cache (x509certificate.ts:91-98) + +### Utils / Conversions + +**HIGH:** + +- [HIGH] Utils — `timingSafeEqual` uses `abvToArrayBuffer` which returns entire backing buffer; TypedArray views compare wrong data (timingSafeEqual.ts:15-16, conversion.ts:17-25) + +**MEDIUM:** + +- [MEDIUM] Utils — `abvToArrayBuffer` does not respect `byteOffset`/`byteLength` for TypedArray views; also used in `timingSafeEqual` (conversion.ts:17-25) +- [MEDIUM] Utils — `binaryLikeToArrayBuffer` duck-typing uses `Symbol.toStringTag === 'KeyObject'`; any object with this tag triggers `.handle.exportKey()` (conversion.ts:142-151) + +**LOW:** + +- [LOW] Utils — `decodeLatin1` does not validate UTF-8 continuation bytes have `10xxxxxx` pattern (HybridUtils.cpp:118-145) + +### Test Coverage Gaps (Key Management & Utilities) + +**HIGH:** + +- [HIGH] Utils — No tests for `timingSafeEqual` with TypedArray views over larger buffers (backing buffer vs view comparison) +- [HIGH] Random — Async randomFill tests swallow exceptions; failures silently ignored (random_tests.ts:53-68,70-85) + +**MEDIUM:** + +- [MEDIUM] KeyObjectHandle — No tests for encrypted private key export/import (cipher/passphrase params untested) +- [MEDIUM] Prime — No negative tests: `generatePrimeSync(0)`, `generatePrimeSync(-1)`, `generatePrimeSync(2)` +- [MEDIUM] KeyObjectHandle — No tests for OKP (Ed25519/X25519) JWK round-trip +- [MEDIUM] Random — No test for `randomUUID` format correctness (version 4 and RFC 4122 variant bits) +- [MEDIUM] X.509 — No tests for malformed certificates, truncated DER, expired certs, critical extensions + +**LOW:** + +- [LOW] Random — No test for `getRandomValues` exceeding 65536 bytes +- [LOW] KeyObjectHandle — No `keyEquals` test for private keys being equal + +### WebCrypto Subtle + +**HIGH:** + +- [HIGH] Subtle — `normalizeAlgorithm` does not perform case-insensitive matching per WebCrypto spec; `"aes-gcm"` bypasses `SUPPORTED_ALGORITHMS` (subtle.ts:86-94) +- [HIGH] Subtle — Key material exported to plaintext via `key.keyObject.export()` for every encrypt/decrypt; "non-extractable" keys transit through JS memory (subtle.ts:261,302,349,477,540) +- [HIGH] Subtle — `deriveBits` accepts `deriveKey` usage as substitute for `deriveBits`; violates spec usage enforcement (subtle.ts:2164-2169) +- [HIGH] Subtle — `hkdfImportKey` does not enforce `extractable === false` per WebCrypto spec (subtle.ts:1583-1603) +- [HIGH] Subtle — `exportKeyRaw` does not enforce key type for symmetric algorithms (subtle.ts:1445-1468) + +**MEDIUM:** + +- [MEDIUM] Subtle — `as unknown as` casts bypass type safety (subtle.ts:2184,2238,2488) +- [MEDIUM] Subtle — Return types `ArrayBuffer | unknown` on export functions; `unknown` union = no type safety (subtle.ts:1282,1346,1408,1477) +- [MEDIUM] Subtle — No AES-GCM IV length validation; spec recommends 12 bytes, disallows 0 (subtle.ts:398-417) +- [MEDIUM] Subtle — RSA import does not validate public key usages vs private key usages (subtle.ts:868-971) +- [MEDIUM] Subtle — JWK import does not validate `jwk.ext` against `extractable` parameter per spec (subtle.ts:897-919,987-1023,1064-1080) +- [MEDIUM] Subtle — JWK import does not validate `jwk.key_ops` against `usages` parameter per spec (subtle.ts:897-919) +- [MEDIUM] Subtle — `cipherOrWrap` switch has no `default`; returns `undefined` typed as `Promise` (subtle.ts:1856-1896) +- [MEDIUM] Subtle — `AnyAlgorithm` includes `'unknown'` as valid value (types.ts:223) +- [MEDIUM] Subtle — `edImportKey` raw format does not restrict usages to public-only (subtle.ts:1156-1164) +- [MEDIUM] Subtle — Enums used despite project rules prohibiting them (subtle.ts:70-79, types.ts:274-298) + +**LOW:** + +- [LOW] Subtle — `EncodingOptions.key` typed as `any` (types.ts:365) +- [LOW] Subtle — Multiple `as` casts without runtime validation: `data as JWK`, `data as BufferLike`, `data as BinaryLike` (subtle.ts:898,922,930,988,1025,1065,1086,1140,1166) +- [LOW] Subtle — Error messages inconsistent: some use `lazyDOMException`, others use plain `new Error()` (subtle.ts:889,892,901,938) +- [LOW] Subtle — `getKeyLength` uses `||` instead of `??`; explicit `0` falls through to default (subtle.ts:2904,2908,2912,2913) +- [LOW] Subtle — `hmacGenerateKey` defaults to 256 bits for unknown hash algorithms silently (subtle.ts:680) + +### Test Coverage Gaps (WebCrypto Subtle) + +**HIGH:** + +- [HIGH] Subtle — No tests for non-extractable key export rejection; `key.extractable` check at line 2283 untested +- [HIGH] Subtle — No cross-algorithm key confusion tests (e.g., AES-GCM key used with AES-CBC) +- [HIGH] Subtle — No tests for JWK `ext` and `key_ops` validation during import +- [HIGH] Subtle — HKDF `extractable: false` enforcement not tested (implementation also doesn't enforce it) +- [HIGH] Subtle — AES/HMAC generateKey tests largely commented out (generateKey.ts:46-68,647-815) + +**MEDIUM:** + +- [MEDIUM] Subtle — No algorithm name case sensitivity tests +- [MEDIUM] Subtle — No negative tests for `deriveBits` with wrong key usage +- [MEDIUM] Subtle — No AES-GCM tests with unusual IV lengths (empty, very long) +- [MEDIUM] Subtle — No RSA key type vs usage mismatch tests during import +- [MEDIUM] Subtle — No wrap/unwrap negative tests (non-extractable key, wrong algorithm, corrupted data) +- [MEDIUM] Subtle — No HKDF `deriveBits`/`deriveKey` tests in subtle test suite + +**LOW:** + +- [LOW] Subtle — `getPublicKey` tests do not cover ML-DSA or ML-KEM +- [LOW] Subtle — No `subtle.supports()` tests for `encapsulateBits`/`decapsulateBits` operations +- [LOW] Subtle — Digest tests do not test error cases (invalid algorithm, null data) + +--- + +## Implementation Plan + +The scan surfaces ~120 HIGH / ~180 MEDIUM / ~50 LOW findings across 32 modules, but the **Recurring Patterns** table collapses these to roughly **15 root causes**. The plan is sequenced so each phase unblocks the next and shared fixes close many findings at once. + +Status key: `[ ]` not started · `[~]` in progress · `[x]` complete + +### Phase 0 — Stop the Bleeding (actively exploitable) + +| # | Status | Issue | Files | Closes | +| --- | ------ | -------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------- | +| 0.1 | [x] | `abvToArrayBuffer` byte-offset bug | `src/utils/{conversion,timingSafeEqual}.ts`, `src/cipher.ts` setAAD, `src/x509certificate.ts` string ctor | `timingSafeEqual` HIGH, all AEAD `setAAD` HIGH, x509 string-input pool-leak (newly found), conversion.ts doc hardening | +| 0.2 | [x] | XSalsa20 keystream restart on every `update()` | `cpp/cipher/XSalsa20Cipher.{cpp,hpp}` | XSalsa20 catastrophic finding | +| 0.3 | [x] | DH/ECDH peer-key validation missing | `cpp/dh/HybridDiffieHellman.cpp`, `cpp/ecdh/HybridECDH.cpp` | DH/ECDH HIGH findings | +| 0.4 | [x] | RSA Bleichenbacher oracle (PKCS#1 v1.5 distinguishable errors) | `cpp/cipher/HybridRsaCipher.cpp`, `src/utils/publicCipher.ts` | RSA Cipher HIGH | + +### Phase 1 — Shared Foundation (root-cause helpers) + +Once these helpers exist the bulk Phase 2/3 sweep just consumes them. + +| # | Status | Helper | Closes | +| --- | ------ | ---------------------------------------------------------------------------- | -------------------------------------------------------------------------- | +| 1.1 | [ ] | `validateDouble()` — reject NaN/Inf/negative/non-integer at JS↔C++ boundary | ~20 findings (Hash, HMAC, KMAC, BLAKE3, all KDFs, RSA, ML-DSA, AES-CCM) | +| 1.2 | [ ] | `secureZero()` — `OPENSSL_cleanse` / `sodium_memzero` wrapper | XSalsa20, XChaCha20-Poly1305, all KDFs, DH/ECDH, RSA/EC/Ed/DSA DER strings | +| 1.3 | [ ] | `EVP_CIPHER_CTX` `unique_ptr` in `HybridCipher` base | CCMCipher, ChaCha20, ChaCha20-Poly1305 destructor leaks | +| 1.4 | [ ] | Replace `Record` `getUIntOption` with typed helper | Cross-cutting cipher options | + +### Phase 2 — Memory Safety Sweep + +Depends on Phase 1. + +- [ ] Raw `new uint8_t[]` → `std::unique_ptr` (Hash, HMAC, KMAC, BLAKE3, all KDFs, all ciphers' `update()`) +- [ ] Raw `EVP_PKEY*` → smart pointers (RSA, EC, Ed, Sign/Verify, ML-DSA, ML-KEM — DSA pattern as template) +- [ ] `Promise::async` raw `this` capture → `shared_from_this` (ML-DSA, ML-KEM, DH) +- [ ] `EVP_PKEY_CTX` double-free in `EVP_DigestSignInit` paths (Sign/Verify, Ed25519, ML-DSA) +- [ ] Ed25519 thread-unsafe `ERR_error_string(.., NULL)` → `ERR_error_string_n` + +### Phase 3 — TypeScript Boundary Validation + +- [ ] Cipher algorithm/key/IV length validation at TS layer +- [ ] KDF parameter validation: Scrypt N power-of-2; HKDF max `255 * HashLen`; Argon2 RFC 9106 mins; PBKDF2 already done — use as template +- [ ] RSA modulus min 2048 bits (currently 256) +- [ ] DSA modulus min 1024 bits (currently 0) +- [ ] WebCrypto `subtle`: case-insensitive `normalizeAlgorithm`; JWK `ext`/`key_ops`; `deriveBits` usage; HKDF `extractable: false` +- [ ] Stream `_transform`/`_flush` error propagation via callback (Hash, HMAC, all ciphers) + +### Phase 4 — Test Vector Coverage + +- [ ] NIST KATs: Hash (SHA family), AES-GCM/CCM/OCB, ML-DSA, ML-KEM +- [ ] RFC vectors: HKDF (RFC 5869, all 7), BLAKE3 keyed_hash/derive_key, Argon2 RFC 9106 with output comparison, Scrypt RFC 7914 Test Case 4 +- [ ] AEAD misuse tests: `setAAD` after `update`, `getAuthTag` on decipher, `setAuthTag` on cipher, missing `setAuthTag` before decrypt +- [ ] Wrong key/IV size rejection tests (every cipher) +- [ ] Fix fire-and-forget async assertions (PBKDF2, Random) +- [ ] Cross-implementation verification (Node.js ↔ RNQC for sigs/KDFs) + +### Phase 5 — Cross-Cutting Audit Items (still unstarted) + +- [ ] `bun audit` on all workspace packages +- [ ] Native dep CVE check (blake3, ncrypto, fastpbkdf2, OpenSSL-Universal, libsodium) +- [ ] GitHub Actions review (injection, secrets exposure) +- [ ] `.npmignore` / published-artifact review (no test fixtures, keys, configs) +- [ ] Expo plugin (`withRNQC`) code-injection review + +--- + +### Progress Log + +_Append entries as PRs land. Format: `YYYY-MM-DD — [phase.task] description (PR #)`_ + +- 2026-04-26 — [0.1] Fix byte-offset bugs across `timingSafeEqual`, AEAD `setAAD`, and X.509 string constructor. Harden `abvToArrayBuffer` doc to flag the zero-copy semantic. Adds 5 regression tests (3 timingSafeEqual view cases, 2 GCM sliced-AAD cases). (branch: `feat/security-audit`, PR: TBD) + - Newly discovered while sweeping: `X509Certificate(string)` was using `Buffer.from(str).buffer` which can return a pool-backed ArrayBuffer with non-zero `byteOffset` — same class of bug as `setAAD`. Fixed in this pass. +- 2026-04-26 — [0.2] Fix XSalsa20 keystream restart on every `update()`. Replace `crypto_stream_xor` with `crypto_stream_xsalsa20_xor_ic` plus per-instance `block_counter` + 64-byte `leftover_keystream` so the keystream advances correctly across chunked update() calls. Output now uses `unique_ptr` for exception safety on the failure path. Adds 6 streaming regression tests covering block-aligned splits, mid-block splits, many-small-chunk splits, drain-to-boundary continuation, the catastrophic two-time-pad regression (identical plaintext in two updates → distinct ciphertexts), and a streaming round-trip across encrypt + decrypt instances. Independent crypto-specialist review approved correctness, exception safety, and re-init isolation. (branch: `feat/security-audit`, PR: TBD) +- 2026-04-26 — [0.3] Add explicit peer-public-key validation in `DiffieHellman::computeSecret` and `ECDH::computeSecret`. DH path calls `DH_check_pub_key` (matching ncrypto's `DHPointer::checkPublicKey`) and distinguishes TOO_SMALL / TOO_LARGE / INVALID error codes, closing the small-subgroup attack on peer pubkeys 0, 1, p-1, and p. ECDH path calls `EC_POINT_oct2point` → `EC_POINT_is_at_infinity` → `EC_POINT_is_on_curve` against the configured group, closing the invalid-curve attack (peer point on a related weaker curve). Adds 4 DH and 5 ECDH regression tests covering each rejection path plus a cross-curve attack (P-384 pubkey sent to a P-256 instance) and a bit-flipped-coordinate test. Crypto-specialist review approved both fixes; flagged that the q-less subgroup gap for caller-supplied DH primes matches Node.js behavior and is not a regression. (branch: `feat/security-audit`, PR: TBD) +- 2026-04-26 — [0.4] Close the RSA PKCS#1 v1.5 Bleichenbacher oracle. `HybridRsaCipher` now (1) enables OpenSSL 3.2+ implicit rejection (`EVP_PKEY_CTX_ctrl_str(ctx, "rsa_pkcs1_implicit_rejection", "1")`) for every PKCS#1 v1.5 decryption — corrupted ciphertexts deterministically decrypt to random-looking bytes instead of throwing — and (2) routes every decrypt-failure path in `decrypt`, `privateDecrypt`, and `publicDecrypt` (verify-recover) through a single `throwOpaqueDecryptFailure()` helper that emits the same `"RSA decryption failed"` message and clears the OpenSSL error stack so the underlying reason never reaches the caller. The TS wrapper drops the `: ${error.message}` interpolation in `privateDecrypt`/`publicDecrypt`. If the OpenSSL build does not support the implicit-rejection knob (BoringSSL or pre-3.2) we hard-fail PKCS#1 v1.5 decryption with a build-config error rather than silently leaving the timing-side oracle open — matches Node.js's `crypto_cipher.cc` policy. Adds 5 regression tests: corrupted PKCS#1 v1.5 doesn't throw, the implicit-rejection output is deterministic per (key, ciphertext) and distinct across different ciphertexts, OAEP/wrong-label errors are opaque (no "openssl/padding/oaep/label" terms in the message), OAEP and PKCS#1 wrong-padding errors are equivalent, and `publicDecrypt` errors are opaque. Crypto-specialist review confirmed the fix is closer to Node-compat than the previous behavior and approved the hard-fail fallback. (branch: `feat/security-audit`, PR: TBD)