Skip to content

Commit 7d9b0a9

Browse files
authored
fix(security): Phase 0 audit fixes — actively exploitable findings (#982)
1 parent 3efab23 commit 7d9b0a9

20 files changed

Lines changed: 2174 additions & 54 deletions
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<rules category="testing">
2+
<rule severity="HIGH" enforcement="STRICT">
3+
<name>Wait for user test confirmation before pushing fixes</name>
4+
<description>
5+
Tests in this project run only in the example React Native app — the
6+
assistant cannot execute them. After fixing a bug or adding a feature
7+
that would normally be validated by running tests, commit locally and
8+
WAIT for the user to manually run the test suite (`bun ios` /
9+
`bun android` and exercise the relevant suite) and confirm it passes
10+
before pushing to the remote.
11+
</description>
12+
<requirements>
13+
<requirement>After committing a fix that requires runtime validation, do NOT immediately `git push`.</requirement>
14+
<requirement>Tell the user the commit is ready and ask them to run the test that exercises the fix.</requirement>
15+
<requirement>Wait for explicit user confirmation ("tests pass", "all green", "ship it", etc.) before pushing.</requirement>
16+
<requirement>If the user reports a failure, iterate locally with new commits — do NOT push interim fixes either.</requirement>
17+
<requirement>Once the user confirms, batch-push all the validated commits in one `git push`.</requirement>
18+
</requirements>
19+
<appliesWhen>
20+
<case>The change touches C++ that only the example app can exercise.</case>
21+
<case>The change modifies behavior that an existing example-app test asserts.</case>
22+
<case>The change adds new example-app tests whose pass/fail is unknown.</case>
23+
<case>A previous push had a confirmed test failure and you are pushing the followup.</case>
24+
</appliesWhen>
25+
<doesNotApplyWhen>
26+
<case>The change is purely documentation, plan files, or `.claude/` config — no runtime impact.</case>
27+
<case>The user explicitly says "push it" or "ship now" before running tests.</case>
28+
<case>The branch has never been pushed and you are creating the first PR push (still ask first if any unverified runtime change is included).</case>
29+
</doesNotApplyWhen>
30+
<rationale>
31+
The /pr and /commit skills are written for projects where the assistant
32+
can run tests itself. This project's testing model puts that step on the
33+
user. Pushing unverified fixes ships potentially broken code to the
34+
remote, pollutes PR history with revert-style "fix the fix" commits,
35+
and burns user trust. The cost of waiting is a single round-trip
36+
message; the cost of not waiting is a force-push or noise on the PR.
37+
</rationale>
38+
<mustAcknowledge>true</mustAcknowledge>
39+
</rule>
40+
</rules>

example/ios/Podfile.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2815,7 +2815,7 @@ SPEC CHECKSUMS:
28152815
NitroMmkv: afbc5b2fbf963be567c6c545aa1efcf6a9cec68e
28162816
NitroModules: 11bba9d065af151eae51e38a6425e04c3b223ff3
28172817
OpenSSL-Universal: 9110d21982bb7e8b22a962b6db56a8aa805afde7
2818-
QuickCrypto: 51001a1827a20257b7c159d8c527306cdb578b5a
2818+
QuickCrypto: f8ed40d88a6dcacc4451d22004c2fd22b8d07f79
28192819
RCT-Folly: 846fda9475e61ec7bcbf8a3fe81edfcaeb090669
28202820
RCTDeprecation: c4b9e2fd0ab200e3af72b013ed6113187c607077
28212821
RCTRequired: e97dd5dafc1db8094e63bc5031e0371f092ae92a

example/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
"react-native-mmkv": "4.0.1",
4141
"react-native-nitro-modules": "0.33.2",
4242
"react-native-quick-base64": "2.2.2",
43-
"react-native-quick-crypto": "1.1.0",
43+
"react-native-quick-crypto": "workspace:*",
4444
"react-native-safe-area-context": "5.6.2",
4545
"react-native-screens": "4.18.0",
4646
"react-native-vector-icons": "10.3.0",

example/src/tests/cipher/cipher_tests.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,3 +399,96 @@ test(SUITE, 'GCM tampered auth tag throws error', () => {
399399

400400
expect(() => decipher.final()).to.throw();
401401
});
402+
403+
// --- setAAD byte-offset regression tests ---
404+
// Pre-fix, setAAD passed `buffer.buffer` to native, ignoring byteOffset /
405+
// byteLength on sliced Buffers. That meant a sliced AAD authenticated the
406+
// wrong bytes — a silent AEAD integrity violation.
407+
408+
test(
409+
SUITE,
410+
'GCM setAAD with sliced Buffer authenticates the slice (not backing)',
411+
() => {
412+
const testKey = Buffer.from(randomFillSync(new Uint8Array(32)));
413+
const testIv = randomFillSync(new Uint8Array(12));
414+
const testPlaintext = Buffer.from('test data for AAD slice');
415+
416+
// Build a backing buffer with a known 16-byte AAD region in the middle and
417+
// distinct surrounding bytes. The cipher must only authenticate the slice.
418+
const backing = Buffer.concat([
419+
Buffer.from('PREFIX_NOISE_'),
420+
Buffer.from('aad-payload-1234'), // 16-byte AAD window
421+
Buffer.from('_SUFFIX_NOISE'),
422+
]);
423+
const aadSlice = backing.subarray(13, 13 + 16);
424+
expect(aadSlice.byteLength).to.equal(16);
425+
expect(aadSlice.toString('utf8')).to.equal('aad-payload-1234');
426+
427+
// Encrypt with the sliced AAD.
428+
const cipher = createCipheriv('aes-256-gcm', testKey, Buffer.from(testIv));
429+
cipher.setAAD(aadSlice);
430+
const encrypted = Buffer.concat([
431+
cipher.update(testPlaintext),
432+
cipher.final(),
433+
]);
434+
const authTag = cipher.getAuthTag();
435+
436+
// Decrypt with a freshly-constructed Buffer carrying the same 16 logical
437+
// bytes — no surrounding noise, byteOffset = 0. If setAAD honors the
438+
// slice on encrypt, this must verify successfully.
439+
const aadStandalone = Buffer.from('aad-payload-1234');
440+
const decipher = createDecipheriv(
441+
'aes-256-gcm',
442+
testKey,
443+
Buffer.from(testIv),
444+
);
445+
decipher.setAAD(aadStandalone);
446+
decipher.setAuthTag(authTag);
447+
const plaintextOut = Buffer.concat([
448+
decipher.update(encrypted),
449+
decipher.final(),
450+
]);
451+
expect(plaintextOut.toString('utf8')).to.equal(
452+
testPlaintext.toString('utf8'),
453+
);
454+
},
455+
);
456+
457+
test(
458+
SUITE,
459+
'GCM setAAD with sliced Buffer rejects wrong AAD on decrypt',
460+
() => {
461+
// Mirror of the previous test but supplies different AAD bytes on decrypt
462+
// — must fail authentication.
463+
const testKey = Buffer.from(randomFillSync(new Uint8Array(32)));
464+
const testIv = randomFillSync(new Uint8Array(12));
465+
const testPlaintext = Buffer.from('test data for AAD slice');
466+
467+
const backing = Buffer.concat([
468+
Buffer.from('PREFIX_NOISE_'),
469+
Buffer.from('aad-payload-1234'),
470+
Buffer.from('_SUFFIX_NOISE'),
471+
]);
472+
const aadSlice = backing.subarray(13, 13 + 16);
473+
474+
const cipher = createCipheriv('aes-256-gcm', testKey, Buffer.from(testIv));
475+
cipher.setAAD(aadSlice);
476+
const encrypted = Buffer.concat([
477+
cipher.update(testPlaintext),
478+
cipher.final(),
479+
]);
480+
const authTag = cipher.getAuthTag();
481+
482+
// Decrypt with WRONG AAD bytes — must throw on final().
483+
const wrongAad = Buffer.from('aad-payload-DIFF');
484+
const decipher = createDecipheriv(
485+
'aes-256-gcm',
486+
testKey,
487+
Buffer.from(testIv),
488+
);
489+
decipher.setAAD(wrongAad);
490+
decipher.setAuthTag(authTag);
491+
decipher.update(encrypted);
492+
expect(() => decipher.final()).to.throw();
493+
},
494+
);

example/src/tests/cipher/xsalsa20_tests.ts

Lines changed: 164 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { Buffer, randomFillSync, xsalsa20 } from 'react-native-quick-crypto';
1+
import {
2+
Buffer,
3+
createCipheriv,
4+
createDecipheriv,
5+
randomFillSync,
6+
xsalsa20,
7+
} from 'react-native-quick-crypto';
28
import { expect } from 'chai';
39
import { test } from '../util';
410

@@ -24,3 +30,160 @@ test(SUITE, 'xsalsa20', () => {
2430
// test decrypted == data
2531
expect(decrypted).eql(data);
2632
});
33+
34+
// --- Streaming regression tests ---
35+
//
36+
// XSalsa20 is a stream cipher: chunked update() calls must advance the
37+
// keystream, NOT restart it from block 0 every time. The previous
38+
// implementation called crypto_stream_xor() on each update(), which restarted
39+
// the keystream and produced a two-time pad if the caller streamed >1 chunk.
40+
//
41+
// These tests pin that fix in place by checking streaming equivalence with
42+
// the one-shot xsalsa20() function, which is the correct reference output.
43+
44+
const STREAM_KEY = Buffer.from(
45+
'a8a7d6a5d4a3d2a1a09f9e9d9c8b8a89a8a7d6a5d4a3d2a1a09f9e9d9c8b8a89',
46+
'hex',
47+
);
48+
const STREAM_NONCE = Buffer.from(
49+
'111213141516171821222324252627283132333435363738',
50+
'hex',
51+
);
52+
53+
// Block-aligned split: two 64-byte chunks (full Salsa20 blocks).
54+
test(SUITE, 'xsalsa20 streaming equivalence — block-aligned split', () => {
55+
const data = Buffer.alloc(128);
56+
for (let i = 0; i < data.length; i++) data[i] = i & 0xff;
57+
58+
const oneShot = xsalsa20(
59+
new Uint8Array(STREAM_KEY),
60+
new Uint8Array(STREAM_NONCE),
61+
new Uint8Array(data),
62+
);
63+
64+
const cipher = createCipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE);
65+
const part1 = cipher.update(data.subarray(0, 64));
66+
const part2 = cipher.update(data.subarray(64));
67+
const streamed = Buffer.concat([part1, part2, cipher.final()]);
68+
69+
expect(new Uint8Array(streamed)).eql(oneShot);
70+
});
71+
72+
// Mid-block split: 30 + 70 bytes, neither chunk is a multiple of 64.
73+
test(SUITE, 'xsalsa20 streaming equivalence — mid-block split', () => {
74+
const data = Buffer.alloc(100);
75+
for (let i = 0; i < data.length; i++) data[i] = (i * 7 + 3) & 0xff;
76+
77+
const oneShot = xsalsa20(
78+
new Uint8Array(STREAM_KEY),
79+
new Uint8Array(STREAM_NONCE),
80+
new Uint8Array(data),
81+
);
82+
83+
const cipher = createCipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE);
84+
const part1 = cipher.update(data.subarray(0, 30));
85+
const part2 = cipher.update(data.subarray(30));
86+
const streamed = Buffer.concat([part1, part2, cipher.final()]);
87+
88+
expect(new Uint8Array(streamed)).eql(oneShot);
89+
});
90+
91+
// Many small chunks crossing several block boundaries.
92+
test(SUITE, 'xsalsa20 streaming equivalence — many small chunks', () => {
93+
const data = Buffer.alloc(257);
94+
for (let i = 0; i < data.length; i++) data[i] = (i * 13 + 5) & 0xff;
95+
96+
const oneShot = xsalsa20(
97+
new Uint8Array(STREAM_KEY),
98+
new Uint8Array(STREAM_NONCE),
99+
new Uint8Array(data),
100+
);
101+
102+
const cipher = createCipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE);
103+
const chunkSizes = [1, 7, 16, 31, 33, 64, 65, 40];
104+
const parts: Buffer[] = [];
105+
let offset = 0;
106+
for (const size of chunkSizes) {
107+
const end = Math.min(offset + size, data.length);
108+
if (end > offset) parts.push(cipher.update(data.subarray(offset, end)));
109+
offset = end;
110+
}
111+
if (offset < data.length) parts.push(cipher.update(data.subarray(offset)));
112+
parts.push(cipher.final());
113+
const streamed = Buffer.concat(parts);
114+
115+
expect(new Uint8Array(streamed)).eql(oneShot);
116+
});
117+
118+
// Regression: identical plaintext in two consecutive update() calls MUST
119+
// produce different ciphertexts because the keystream advances. The previous
120+
// (buggy) implementation reset the keystream on every update(), so both
121+
// chunks would have been bitwise identical — a two-time-pad break.
122+
test(SUITE, 'xsalsa20 keystream advances across update() calls', () => {
123+
const block = Buffer.alloc(64, 0xaa);
124+
125+
const cipher = createCipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE);
126+
const c1 = cipher.update(block);
127+
const c2 = cipher.update(block);
128+
cipher.final();
129+
130+
expect(c1.length).to.equal(block.length);
131+
expect(c2.length).to.equal(block.length);
132+
// If the bug returns, c1 === c2 (catastrophic).
133+
expect(c1.equals(c2)).to.equal(false);
134+
});
135+
136+
// Edge case: a chunk that exactly drains the leftover keystream to the block
137+
// boundary, followed by a subsequent update. Catches a regression where
138+
// `leftover_offset` doesn't wrap to the sentinel correctly.
139+
test(
140+
SUITE,
141+
'xsalsa20 streaming equivalence — drain-to-boundary then continue',
142+
() => {
143+
// 60 + 4 + 100 = 164 bytes. After the 60-byte chunk, leftover_offset=60;
144+
// the 4-byte chunk drains exactly to 64 (sentinel); the 100-byte chunk
145+
// must then start cleanly on a fresh block boundary.
146+
const data = Buffer.alloc(164);
147+
for (let i = 0; i < data.length; i++) data[i] = (i * 5 + 19) & 0xff;
148+
149+
const oneShot = xsalsa20(
150+
new Uint8Array(STREAM_KEY),
151+
new Uint8Array(STREAM_NONCE),
152+
new Uint8Array(data),
153+
);
154+
155+
const cipher = createCipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE);
156+
const part1 = cipher.update(data.subarray(0, 60));
157+
const part2 = cipher.update(data.subarray(60, 64));
158+
const part3 = cipher.update(data.subarray(64));
159+
const streamed = Buffer.concat([part1, part2, part3, cipher.final()]);
160+
161+
expect(new Uint8Array(streamed)).eql(oneShot);
162+
},
163+
);
164+
165+
// Streaming round-trip: encrypt and decrypt streamed across multiple
166+
// update() calls. Decryption is just XOR with the same keystream, so this
167+
// also exercises the streaming state on the decrypt side.
168+
test(SUITE, 'xsalsa20 streaming round-trip across two cipher instances', () => {
169+
const data = Buffer.alloc(200);
170+
for (let i = 0; i < data.length; i++) data[i] = (i * 11 + 17) & 0xff;
171+
172+
const enc = createCipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE);
173+
const ciphertext = Buffer.concat([
174+
enc.update(data.subarray(0, 50)),
175+
enc.update(data.subarray(50, 130)),
176+
enc.update(data.subarray(130)),
177+
enc.final(),
178+
]);
179+
180+
const dec = createDecipheriv('xsalsa20', STREAM_KEY, STREAM_NONCE);
181+
const decrypted = Buffer.concat([
182+
dec.update(ciphertext.subarray(0, 17)),
183+
dec.update(ciphertext.subarray(17, 99)),
184+
dec.update(ciphertext.subarray(99)),
185+
dec.final(),
186+
]);
187+
188+
expect(decrypted.equals(data)).to.equal(true);
189+
});

example/src/tests/dh/dh_tests.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,3 +144,45 @@ test(SUITE, 'verifyError should return 0 for created DH', () => {
144144
const dh = crypto.createDiffieHellman(prime, 2);
145145
assert.strictEqual(dh.verifyError, 0);
146146
});
147+
148+
// --- Peer public-key validation (security audit Phase 0.3) ---
149+
//
150+
// Without an explicit DH_check_pub_key call, EVP_PKEY_derive_set_peer
151+
// silently accepts a peer pubkey of 0, 1, or p-1 and produces a
152+
// "shared secret" of 0, 1, or +/-1 — the small-subgroup attack.
153+
154+
test(SUITE, 'computeSecret should reject peer public key of 0', () => {
155+
const alice = crypto.getDiffieHellman('modp14');
156+
alice.generateKeys();
157+
assert.throws(() => {
158+
alice.computeSecret(Buffer.from([0]));
159+
}, /too small/i);
160+
});
161+
162+
test(SUITE, 'computeSecret should reject peer public key of 1', () => {
163+
const alice = crypto.getDiffieHellman('modp14');
164+
alice.generateKeys();
165+
assert.throws(() => {
166+
alice.computeSecret(Buffer.from([1]));
167+
}, /too small/i);
168+
});
169+
170+
test(SUITE, 'computeSecret should reject peer public key of p-1', () => {
171+
const alice = crypto.getDiffieHellman('modp14');
172+
alice.generateKeys();
173+
// modp14 prime ends in 0xFF...FF, so p-1 differs only in the trailing byte.
174+
const pMinus1 = Buffer.from(MODP14_PRIME, 'hex');
175+
pMinus1[pMinus1.length - 1] = pMinus1[pMinus1.length - 1]! ^ 0x01;
176+
assert.throws(() => {
177+
alice.computeSecret(pMinus1);
178+
}, /too large/i);
179+
});
180+
181+
test(SUITE, 'computeSecret should reject peer public key equal to p', () => {
182+
const alice = crypto.getDiffieHellman('modp14');
183+
alice.generateKeys();
184+
const p = Buffer.from(MODP14_PRIME, 'hex');
185+
assert.throws(() => {
186+
alice.computeSecret(p);
187+
}, /too large|invalid/i);
188+
});

0 commit comments

Comments
 (0)