From f99a86579a828516b0b2dc37454f874e38b9f38e Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Sun, 26 Apr 2026 20:17:44 -0400 Subject: [PATCH 1/8] fix(security): correct byte-offset handling in setAAD, timingSafeEqual, X.509 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes Phase 0.1 of the security audit. Three call sites passed the full backing ArrayBuffer to native code, ignoring view byteOffset/byteLength: - timingSafeEqual: switched from abvToArrayBuffer to binaryLikeToArrayBuffer so TypedArray / Buffer views are sliced to their window. Previously compared the entire backing, which both walked unrelated bytes and spuriously failed the byte-length check for any view smaller than its backing. - Cipher.setAAD: stopped passing buffer.buffer; now slices via binaryLikeToArrayBuffer. Sliced AAD silently authenticated the wrong bytes — a direct AEAD integrity violation across GCM/CCM/OCB/Poly1305. - X509Certificate(string): Buffer.from(str).buffer can return a pool-backed ArrayBuffer with byteOffset > 0, exposing unrelated pool bytes to native. Now routes through binaryLikeToArrayBuffer. abvToArrayBuffer's doc-comment is hardened to flag the zero-copy backing semantic and steer future callers toward binaryLikeToArrayBuffer for crypto data paths. randomFill's intentional use of the zero-copy path is preserved. Adds 5 regression tests: 3 timingSafeEqual view cases (offset views with matching content, offset views with differing content, view-vs-view across differently-sized backings) and 2 GCM sliced-AAD cases (verifies the slice is what gets authenticated, and that wrong-AAD on decrypt still throws). Also adds the implementation plan to plans/todo/security-audit.md, which sequences the remaining audit findings into 6 phases. --- example/src/tests/cipher/cipher_tests.ts | 93 ++ example/src/tests/utils/utils_tests.ts | 54 + .../react-native-quick-crypto/src/cipher.ts | 9 +- .../src/utils/conversion.ts | 15 +- .../src/utils/timingSafeEqual.ts | 11 +- .../src/x509certificate.ts | 11 +- plans/todo/security-audit.md | 1134 +++++++++++++++++ 7 files changed, 1311 insertions(+), 16 deletions(-) create mode 100644 plans/todo/security-audit.md 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/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/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/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..0a9f7a44 --- /dev/null +++ b/plans/todo/security-audit.md @@ -0,0 +1,1134 @@ +# 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 | [ ] | XSalsa20 keystream restart on every `update()` | `cpp/cipher/XSalsa20Cipher.{cpp,hpp}` | XSalsa20 catastrophic finding | +| 0.3 | [ ] | DH/ECDH peer-key validation missing | `cpp/dh/HybridDiffieHellman.cpp`, `cpp/ecdh/HybridECDH.cpp` | DH/ECDH HIGH findings | +| 0.4 | [ ] | 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. + From 61d00aa2a6cb2663ad8078819dff51bcf9aad386 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Sun, 26 Apr 2026 20:36:59 -0400 Subject: [PATCH 2/8] fix(security): advance XSalsa20 keystream across update() calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous implementation called crypto_stream_xor() on every update(), which restarts the XSalsa20 keystream at block 0. Streaming a message in N chunks therefore produced N copies of the same keystream XORed against different plaintext — a textbook two-time pad. Any caller using the streaming Cipheriv API for XSalsa20 was catastrophically vulnerable. Switch to crypto_stream_xsalsa20_xor_ic() with an explicit block counter, plus a 64-byte leftover-keystream buffer so partial-block chunks can resume cleanly on the next update(). init() resets all streaming state so re-init is safe. Output now uses unique_ptr / release() to avoid leaking on the failure path. Adds 6 streaming regression tests: block-aligned splits, mid-block splits, many-small-chunk splits, drain-to-boundary continuation, the two-time-pad regression (same plaintext → distinct ciphertexts), and a streaming round-trip across separate encrypt/decrypt instances. Closes Phase 0.2 in plans/todo/security-audit.md. --- example/src/tests/cipher/xsalsa20_tests.ts | 165 +++++++- .../cpp/cipher/XSalsa20Cipher.cpp | 78 +++- .../cpp/cipher/XSalsa20Cipher.hpp | 16 + plans/todo/security-audit.md | 357 ++++++++++++------ 4 files changed, 491 insertions(+), 125 deletions(-) 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/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/plans/todo/security-audit.md b/plans/todo/security-audit.md index 0a9f7a44..8e0898d0 100644 --- a/plans/todo/security-audit.md +++ b/plans/todo/security-audit.md @@ -112,84 +112,84 @@ Each module is audited by all relevant agents. Status key: ### 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | --- @@ -230,53 +230,53 @@ Move this file to `plans/done/` when the full audit is complete. 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. | +| 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. | --- @@ -287,6 +287,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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) @@ -297,6 +298,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -308,6 +310,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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 @@ -316,6 +319,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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) @@ -325,6 +329,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -337,6 +342,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -345,6 +351,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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) @@ -355,6 +362,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -367,6 +375,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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 @@ -376,6 +385,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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) @@ -385,6 +395,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -395,6 +406,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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 @@ -403,12 +415,14 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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) @@ -416,6 +430,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -423,27 +438,32 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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) @@ -451,17 +471,20 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -472,94 +495,113 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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) @@ -568,6 +610,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -575,11 +618,13 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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) @@ -587,12 +632,14 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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 @@ -611,6 +658,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -635,6 +683,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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 @@ -647,46 +696,54 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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) @@ -694,29 +751,34 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -725,23 +787,27 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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] 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) @@ -749,6 +815,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -756,11 +823,13 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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) @@ -768,6 +837,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -775,11 +845,13 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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` @@ -788,84 +860,101 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -874,6 +963,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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`) @@ -882,10 +972,12 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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) @@ -893,6 +985,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -900,10 +993,12 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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) @@ -911,17 +1006,20 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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 @@ -929,76 +1027,92 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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 @@ -1006,12 +1120,14 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -1019,6 +1135,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -1031,6 +1148,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -1040,6 +1158,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr ### 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 @@ -1047,6 +1166,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -1055,6 +1175,7 @@ _Record issues here as they are discovered. Format: `[severity] module — descr - [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) @@ -1069,23 +1190,23 @@ 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 | [ ] | XSalsa20 keystream restart on every `update()` | `cpp/cipher/XSalsa20Cipher.{cpp,hpp}` | XSalsa20 catastrophic finding | -| 0.3 | [ ] | DH/ECDH peer-key validation missing | `cpp/dh/HybridDiffieHellman.cpp`, `cpp/ecdh/HybridECDH.cpp` | DH/ECDH HIGH findings | -| 0.4 | [ ] | RSA Bleichenbacher oracle (PKCS#1 v1.5 distinguishable errors) | `cpp/cipher/HybridRsaCipher.cpp`, `src/utils/publicCipher.ts` | RSA Cipher HIGH | +| # | 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 | [ ] | DH/ECDH peer-key validation missing | `cpp/dh/HybridDiffieHellman.cpp`, `cpp/ecdh/HybridECDH.cpp` | DH/ECDH HIGH findings | +| 0.4 | [ ] | 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 | +| # | 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 @@ -1131,4 +1252,4 @@ _Append entries as PRs land. Format: `YYYY-MM-DD — [phase.task] description (P - 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) From 827f0a17d3fe5cc260b36b72ef1d51297678b70f Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Sun, 26 Apr 2026 20:48:01 -0400 Subject: [PATCH 3/8] fix(security): validate peer public keys in DH/ECDH computeSecret MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DH and ECDH both relied on EVP_PKEY_derive_set_peer() to "validate" the peer key, but that EVP entrypoint does not call DH_check_pub_key or EVP_PKEY_public_check, so: - DH silently accepted peer pubkeys of 0, 1, or p-1 and produced a "shared secret" of 0, 1, or +/-1 — the small-subgroup attack. - ECDH silently accepted peer points on a related/weaker curve, enabling the invalid-curve attack and leaking bits of our private key in the resulting shared secret. Add explicit validation up front in both computeSecret paths: - DH: BN_bin2bn the peer key, call DH_check_pub_key against our DH parameters, and surface TOO_SMALL / TOO_LARGE / INVALID flags as distinct error messages (matching ncrypto's DHPointer::checkPublicKey). - ECDH: EC_POINT_oct2point against the configured group, then EC_POINT_is_at_infinity (reject identity), then EC_POINT_is_on_curve (defensive — oct2point validates internally on OpenSSL 3.x but a second check protects against regressions and partial implementations). Adds 4 DH and 5 ECDH regression tests covering each rejection path, including a cross-curve test (P-384 pubkey sent to a P-256 instance) and a bit-flipped-coordinate test that exercises is_on_curve. Closes Phase 0.3 in plans/todo/security-audit.md. --- example/src/tests/dh/dh_tests.ts | 42 +++++++++++ example/src/tests/ecdh/ecdh_tests.ts | 69 +++++++++++++++++++ .../cpp/dh/HybridDiffieHellman.cpp | 29 ++++++++ .../cpp/ecdh/HybridECDH.cpp | 23 +++++++ plans/todo/security-audit.md | 3 +- 5 files changed, 165 insertions(+), 1 deletion(-) 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/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/plans/todo/security-audit.md b/plans/todo/security-audit.md index 8e0898d0..95982369 100644 --- a/plans/todo/security-audit.md +++ b/plans/todo/security-audit.md @@ -1194,7 +1194,7 @@ Status key: `[ ]` not started · `[~]` in progress · `[x]` complete | --- | ------ | -------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------- | | 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 | [ ] | DH/ECDH peer-key validation missing | `cpp/dh/HybridDiffieHellman.cpp`, `cpp/ecdh/HybridECDH.cpp` | DH/ECDH HIGH findings | +| 0.3 | [x] | DH/ECDH peer-key validation missing | `cpp/dh/HybridDiffieHellman.cpp`, `cpp/ecdh/HybridECDH.cpp` | DH/ECDH HIGH findings | | 0.4 | [ ] | 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) @@ -1253,3 +1253,4 @@ _Append entries as PRs land. Format: `YYYY-MM-DD — [phase.task] description (P - 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) From 82c6ac1f1e8fbf99f000380b38939bd3c436eaa4 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Sun, 26 Apr 2026 21:01:06 -0400 Subject: [PATCH 4/8] fix(security): close RSA PKCS#1 v1.5 Bleichenbacher oracle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RSA decryption surfaced OpenSSL error strings verbatim through both the C++ and TS layers, so a remote attacker could distinguish "padding invalid" from "data too large", "bad version", "wrong key", etc. — a classic Million Message Attack padding oracle. Two-layer fix: 1. Implicit rejection. For PKCS#1 v1.5 decryption, enable OpenSSL 3.2+ implicit rejection via EVP_PKEY_CTX_ctrl_str(ctx, "rsa_pkcs1_implicit_rejection", "1"). Corrupted ciphertexts now deterministically decrypt to random-looking bytes instead of throwing. If the underlying OpenSSL build does not support the knob (BoringSSL, 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. 2. Opaque errors. Every decrypt-failure path in decrypt(), privateDecrypt(), and publicDecrypt() (verify-recover) routes 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 and just throws "privateDecrypt failed" / "publicDecrypt failed". 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); OAEP and PKCS#1 wrong-padding errors are equivalent; publicDecrypt errors are opaque. Closes Phase 0.4 in plans/todo/security-audit.md. --- example/src/tests/keys/public_cipher.ts | 199 ++++++++++++++++++ .../cpp/cipher/HybridRsaCipher.cpp | 88 +++++--- .../src/keys/publicCipher.ts | 14 +- plans/todo/security-audit.md | 3 +- 4 files changed, 267 insertions(+), 37 deletions(-) diff --git a/example/src/tests/keys/public_cipher.ts b/example/src/tests/keys/public_cipher.ts index dd827a67..3abc08c2 100644 --- a/example/src/tests/keys/public_cipher.ts +++ b/example/src/tests/keys/public_cipher.ts @@ -1069,3 +1069,202 @@ 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 → throw must NOT leak OpenSSL error reasons. + let caught: Error | undefined; + try { + publicDecrypt(publicKey, Buffer.alloc(256, 0xcd)); + } catch (e) { + caught = e as Error; + } + expect(caught).to.be.instanceOf(Error); + expect(caught!.message).to.equal('publicDecrypt failed'); + expect(caught!.message.toLowerCase()).to.not.match( + /openssl|padding|pkcs|version|recover|verify/, + ); + }, +); diff --git a/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp b/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp index 703fffb8..b25ec048 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,8 @@ 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) { - 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 +391,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 +429,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/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/plans/todo/security-audit.md b/plans/todo/security-audit.md index 95982369..f442ff4c 100644 --- a/plans/todo/security-audit.md +++ b/plans/todo/security-audit.md @@ -1195,7 +1195,7 @@ Status key: `[ ]` not started · `[~]` in progress · `[x]` complete | 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 | [ ] | RSA Bleichenbacher oracle (PKCS#1 v1.5 distinguishable errors) | `cpp/cipher/HybridRsaCipher.cpp`, `src/utils/publicCipher.ts` | RSA Cipher HIGH | +| 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) @@ -1254,3 +1254,4 @@ _Append entries as PRs land. Format: `YYYY-MM-DD — [phase.task] description (P - 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) From 94a77e641365c60f4c4c603df04704d3859f42da Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Sun, 26 Apr 2026 21:19:10 -0400 Subject: [PATCH 5/8] fix(security): restore RSA publicDecrypt empty-plaintext recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 0.4 collapsed every decrypt-failure path through the new opaque error helper, which removed the special case in publicDecrypt's second verify_recover call that returned an empty buffer when the recovered payload was zero bytes. That broke `publicDecrypt(privateEncrypt(""))` round-trips — the existing test "privateEncrypt/publicDecrypt empty plaintext" now reports "publicDecrypt failed". Restore the narrow OpenSSL-error-code match so empty-plaintext recovery works again. publicDecrypt is signature verification with the PUBLIC key (anyone can perform it), so this special case is not a Bleichenbacher target. The fall-through still routes through the opaque-error helper when no match is hit. Loosen the new "Bleichenbacher: publicDecrypt errors are opaque" test to accept either the silent-empty path or the opaque-throw path — both are info-leak-free. The throw branch still asserts the message contains no OpenSSL details. Note: the broader "masks real failures" concern flagged in the audit as MEDIUM (HybridRsaCipher.cpp:264) is preserved as a separate finding to be tightened in a later pass with a precise OpenSSL 3.x reason-code match. --- example/src/tests/keys/public_cipher.ts | 18 ++++++++++++------ .../cpp/cipher/HybridRsaCipher.cpp | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/example/src/tests/keys/public_cipher.ts b/example/src/tests/keys/public_cipher.ts index 3abc08c2..72bc704f 100644 --- a/example/src/tests/keys/public_cipher.ts +++ b/example/src/tests/keys/public_cipher.ts @@ -1254,17 +1254,23 @@ test( type: 'spki', }); - // Garbage in → throw must NOT leak OpenSSL error reasons. + // 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; } - expect(caught).to.be.instanceOf(Error); - expect(caught!.message).to.equal('publicDecrypt failed'); - expect(caught!.message.toLowerCase()).to.not.match( - /openssl|padding|pkcs|version|recover|verify/, - ); + 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/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp b/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp index b25ec048..62185107 100644 --- a/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp +++ b/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp @@ -290,6 +290,21 @@ 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. + unsigned long err = ERR_peek_last_error(); + 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); throwOpaqueDecryptFailure(); } From 6d654c428a0325d7be9be32a343133fa01ae5311 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Sun, 26 Apr 2026 21:23:58 -0400 Subject: [PATCH 6/8] fix(security): use ERR_get_error (oldest) for RSA empty-recovery match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous fixup used ERR_peek_last_error which returns the NEWEST error in OpenSSL's FIFO queue. For verify_recover failures the queue typically holds an outer wrapper error on top of the inner padding-check error, so the narrow constants from the original code (0x1C880004, low-byte 0x04) never matched and every recovery went through throwOpaqueDecryptFailure. Switch to ERR_get_error to read the OLDEST queued error — same behavior as the original code that worked, restoring the empty plaintext round-trip. --- .../cpp/cipher/HybridRsaCipher.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp b/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp index 62185107..5801c50f 100644 --- a/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp +++ b/packages/react-native-quick-crypto/cpp/cipher/HybridRsaCipher.cpp @@ -298,7 +298,11 @@ std::shared_ptr HybridRsaCipher::publicDecrypt(const std::shared_pt // 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. - unsigned long err = ERR_peek_last_error(); + // + // 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(); if ((err & 0xFFFFFFF) == 0x1C880004 || (err & 0xFF) == 0x04) { ERR_clear_error(); EVP_PKEY_CTX_free(ctx); From 024ba285e74faba3c2a52391d831c87a1828454a Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Sun, 26 Apr 2026 21:28:13 -0400 Subject: [PATCH 7/8] chore(claude): require user test confirmation before pushing fixes Tests in this project run only in the example React Native app and the assistant cannot execute them. The default /commit and /pr skills both push immediately after committing, which has produced a "fix the fix" loop in this very session: a runtime regression shipped to the remote PR, was found by the user, the fix was committed and pushed without verification, and the fix-of-the-fix had to be pushed too. Codify the implicit norm as a HIGH/STRICT rule: after any commit that needs runtime validation in the example app, wait for the user to run the relevant test suite and explicitly confirm it passes before pushing. Iterate locally on failure; batch the validated commits into a single push when the user gives the go-ahead. --- .claude/rules/test-verification.xml | 40 +++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 .claude/rules/test-verification.xml 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 + + From 06c2304bc92f0a5c94e68c66fdc1252525107c23 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Sun, 26 Apr 2026 21:28:33 -0400 Subject: [PATCH 8/8] chore(example): use workspace dep + refresh Podfile.lock Switch the example app from the published "1.1.0" version of react-native-quick-crypto to "workspace:*" so local C++ / TS edits land on the next bun ios / bun android without a publish round-trip. Refresh Podfile.lock with the new QuickCrypto pod hash that pops out of the Phase 0 C++ changes. --- example/ios/Podfile.lock | 2 +- example/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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",