Skip to content

Commit 7766d42

Browse files
committed
feat(security): replace Record<string, any> getUIntOption with typed helper
The previous getUIntOption signature defeated the type checker: function getUIntOption(options: Record<string, any>, key: string) `any` everywhere meant the value extracted by `options[key]` was typed as `any` and passed to the cipher constructor with zero validation beyond the cryptic `value >>> 0 !== value` check. The function also returned `-1` as a sentinel for "missing", forcing every caller into the awkward `getUIntOption(...) !== -1 ? getUIntOption(...) : default` pattern (which double-evaluates). Replace with a typed helper: - Input: `Readonly<Record<string, unknown>> | undefined` — narrows from `any` to `unknown`, requiring runtime checks before use. - Return: `number | undefined` — the missing case is type-encoded. - Validation: explicit `Number.isFinite/Integer + range [0, 2^32-1]` with a typed `RangeError` and a descriptive message. The single call site collapses from a 4-line conditional to: const authTagLen = getUIntOption(options, 'authTagLength') ?? 16; Adds 4 regression tests covering negative, NaN, fractional, and missing-defaults-to-16 paths. Phase 1.4 of plans/todo/security-audit.md.
1 parent 73ea9f3 commit 7766d42

3 files changed

Lines changed: 81 additions & 12 deletions

File tree

example/src/tests/cipher/cipher_tests.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,3 +492,48 @@ test(
492492
expect(() => decipher.final()).to.throw();
493493
},
494494
);
495+
496+
// --- getUIntOption type-safety regression (Phase 1.4) ---
497+
//
498+
// Ensure the AEAD `authTagLength` option is validated at the JS boundary.
499+
// The previous implementation used `Record<string, any>` and the cryptic
500+
// `value >>> 0 !== value` check; the typed replacement throws RangeError
501+
// with a clear "must be a non-negative 32-bit integer" message.
502+
503+
test(SUITE, 'createCipheriv: rejects negative authTagLength', () => {
504+
expect(() => {
505+
createCipheriv('aes-256-gcm', key32, iv12, {
506+
authTagLength: -1,
507+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
508+
} as any);
509+
}).to.throw(/non-negative/i);
510+
});
511+
512+
test(SUITE, 'createCipheriv: rejects NaN authTagLength', () => {
513+
expect(() => {
514+
createCipheriv('aes-256-gcm', key32, iv12, {
515+
authTagLength: NaN,
516+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
517+
} as any);
518+
}).to.throw(/non-negative/i);
519+
});
520+
521+
test(SUITE, 'createCipheriv: rejects fractional authTagLength', () => {
522+
expect(() => {
523+
createCipheriv('aes-256-gcm', key32, iv12, {
524+
authTagLength: 12.5,
525+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
526+
} as any);
527+
}).to.throw(/non-negative/i);
528+
});
529+
530+
test(
531+
SUITE,
532+
'createCipheriv: missing authTagLength still defaults to 16',
533+
() => {
534+
// Sanity check that the new helper's `?? 16` default still kicks in.
535+
expect(() => {
536+
createCipheriv('aes-256-gcm', key32, iv12, {});
537+
}).to.not.throw();
538+
},
539+
);

packages/react-native-quick-crypto/src/cipher.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,12 @@ class CipherCommon extends Stream.Transform {
107107
}
108108
super(streamOptions); // Pass filtered options
109109

110-
const authTagLen: number =
111-
getUIntOption(options ?? {}, 'authTagLength') !== -1
112-
? getUIntOption(options ?? {}, 'authTagLength')
113-
: 16; // defaults to 16 bytes
110+
// defaults to 16 bytes for AEAD modes; non-AEAD callers ignore it.
111+
const authTagLen =
112+
getUIntOption(
113+
options as Readonly<Record<string, unknown>> | undefined,
114+
'authTagLength',
115+
) ?? 16;
114116

115117
const factory =
116118
NitroModules.createHybridObject<CipherFactory>('CipherFactory');

packages/react-native-quick-crypto/src/utils/cipher.ts

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,35 @@ export function validateEncoding(data: string, encoding: string) {
4848
}
4949
}
5050

51-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
52-
export function getUIntOption(options: Record<string, any>, key: string) {
53-
let value;
54-
if (options && (value = options[key]) != null) {
55-
// >>> Turns any type into a positive integer (also sets the sign bit to 0)
56-
if (value >>> 0 !== value) throw new Error(`options.${key}: ${value}`);
57-
return value;
51+
/**
52+
* Reads an unsigned-integer option from an options-like object.
53+
*
54+
* Returns `undefined` if the option is missing, `null`, or `undefined`.
55+
* Throws `RangeError` if the value is present but not a non-negative
56+
* 32-bit integer (NaN, Infinity, fractional, negative, or > 2^32 - 1).
57+
*
58+
* Replaces the previous `Record<string, any>` + sentinel-`-1` signature,
59+
* which defeated the type checker (audit Phase 1.4). Callers that used
60+
* `getUIntOption(opts ?? {}, key) !== -1 ? getUIntOption(...) : default`
61+
* collapse to `getUIntOption(opts, key) ?? default`.
62+
*/
63+
export function getUIntOption(
64+
options: Readonly<Record<string, unknown>> | undefined,
65+
key: string,
66+
): number | undefined {
67+
if (options == null) return undefined;
68+
const value = options[key];
69+
if (value == null) return undefined;
70+
if (
71+
typeof value !== 'number' ||
72+
!Number.isFinite(value) ||
73+
!Number.isInteger(value) ||
74+
value < 0 ||
75+
value > 0xffff_ffff
76+
) {
77+
throw new RangeError(
78+
`options.${key} must be a non-negative 32-bit integer, got ${String(value)}`,
79+
);
5880
}
59-
return -1;
81+
return value;
6082
}

0 commit comments

Comments
 (0)