Skip to content

Commit 33f9fa2

Browse files
ZaneHannanAUZane Hannan
authored andcommitted
crypto,errors: use byteLength, not length in safeTimingEqual
crypto.timingSafeEqual() can cause the core to abort if the length parameter matches; however the internal byte length differs. This commit makes the length validation use bytewise (ArrayBufferLike) byteLength rather than array content length. Reissuing of nodejs#21397 with various modifications and fixes.
1 parent 43a1bc3 commit 33f9fa2

File tree

3 files changed

+105
-3
lines changed

3 files changed

+105
-3
lines changed

lib/internal/crypto/util.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ function timingSafeEqual(buf1, buf2) {
9292
throw new ERR_INVALID_ARG_TYPE('buf2',
9393
['Buffer', 'TypedArray', 'DataView'], buf2);
9494
}
95-
if (buf1.length !== buf2.length) {
95+
if (buf1.byteLength !== buf2.byteLength) {
9696
throw new ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH();
9797
}
9898
return _timingSafeEqual(buf1, buf2);

lib/internal/errors.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ E('ERR_CRYPTO_SCRYPT_NOT_SUPPORTED', 'Scrypt algorithm not supported', Error);
518518
// Switch to TypeError. The current implementation does not seem right.
519519
E('ERR_CRYPTO_SIGN_KEY_REQUIRED', 'No key provided to sign', Error);
520520
E('ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH',
521-
'Input buffers must have the same length', RangeError);
521+
'Input buffers must have the same byteLength', RangeError);
522522
E('ERR_DNS_SET_SERVERS_FAILED', 'c-ares failed to set servers: "%s" [%s]',
523523
Error);
524524
E('ERR_DOMAIN_CALLBACK_NOT_AVAILABLE',

test/sequential/test-crypto-timing-safe-equal.js

Lines changed: 103 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,114 @@ assert.strictEqual(
1818
false
1919
);
2020

21+
{
22+
const ab32 = new ArrayBuffer(32);
23+
const dv32 = new DataView(ab32);
24+
dv32.setUint32(0, 1);
25+
dv32.setUint32(4, 1, true);
26+
dv32.setBigUint64(8, 1n);
27+
dv32.setBigUint64(16, 1n, true);
28+
dv32.setUint16(24, 1);
29+
dv32.setUint16(26, 1, true);
30+
dv32.setUint8(28, 1);
31+
dv32.setUint8(29, 1);
32+
33+
// 'should consider equal buffers to be equal'
34+
assert.strictEqual(
35+
crypto.timingSafeEqual(Buffer.from(ab32), dv32),
36+
true
37+
);
38+
assert.strictEqual(
39+
crypto.timingSafeEqual(new Uint32Array(ab32), dv32),
40+
true
41+
);
42+
assert.strictEqual(
43+
crypto.timingSafeEqual(
44+
new Uint8Array(ab32, 0, 16),
45+
Buffer.from(ab32, 0, 16)
46+
),
47+
true
48+
);
49+
assert.strictEqual(
50+
crypto.timingSafeEqual(
51+
new Uint32Array(ab32, 0, 8),
52+
Buffer.of(0, 0, 0, 1, // 4
53+
1, 0, 0, 0, // 8
54+
0, 0, 0, 0, 0, 0, 0, 1, // 16
55+
1, 0, 0, 0, 0, 0, 0, 0, // 24
56+
0, 1, // 26
57+
1, 0, // 28
58+
1, 1, // 30
59+
0, 0) // 32
60+
),
61+
true
62+
);
63+
64+
// 'should consider unequal buffer views to be unequal'
65+
assert.strictEqual(
66+
crypto.timingSafeEqual(
67+
new Uint32Array(ab32, 16, 4),
68+
Buffer.from(ab32, 0, 16)
69+
),
70+
false
71+
);
72+
assert.strictEqual(
73+
crypto.timingSafeEqual(
74+
new Uint8Array(ab32, 0, 16),
75+
Buffer.from(ab32, 16, 16)
76+
),
77+
false
78+
);
79+
assert.strictEqual(
80+
crypto.timingSafeEqual(
81+
new Uint32Array(ab32, 0, 8),
82+
Buffer.of(0, 0, 0, 1, // 4
83+
1, 0, 0, 0, // 8
84+
0, 0, 0, 0, 0, 0, 0, 1, // 16
85+
1, 0, 0, 0, 0, 0, 0, 0, // 24
86+
0, 1, // 26
87+
1, 0, // 28
88+
1, 1, // 30
89+
0, 1) // 32
90+
),
91+
false
92+
);
93+
// 'buffers with differing byteLength must throw an equal length error'
94+
common.expectsError(
95+
() => crypto.timingSafeEqual(Buffer.from(ab32, 0, 8),
96+
Buffer.from(ab32, 0, 9)),
97+
{
98+
code: 'ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH',
99+
type: RangeError,
100+
message: 'Input buffers must have the same byteLength'
101+
}
102+
);
103+
common.expectsError(
104+
() => crypto.timingSafeEqual(
105+
new Uint32Array(ab32, 0, 8), // 32
106+
Buffer.of(0, 0, 0, 1, // 4
107+
1, 0, 0, 0, // 8
108+
0, 0, 0, 0, 0, 0, 0, 1, // 16
109+
1, 0, 0, 0, 0, 0, 0, 0, // 24
110+
0, 1, // 26
111+
1, 0, // 28
112+
1, 1, // 30
113+
0) // 31
114+
),
115+
{
116+
code: 'ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH',
117+
type: RangeError,
118+
message: 'Input buffers must have the same byteLength'
119+
}
120+
);
121+
}
122+
21123
common.expectsError(
22124
() => crypto.timingSafeEqual(Buffer.from([1, 2, 3]), Buffer.from([1, 2])),
23125
{
24126
code: 'ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH',
25127
type: RangeError,
26-
message: 'Input buffers must have the same length'
128+
message: 'Input buffers must have the same byteLength'
27129
}
28130
);
29131

0 commit comments

Comments
 (0)