Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ad376c3

Browse filesBrowse files
joyeecheungaduh95
joyeecheung
authored and
aduh95
committed
crypto: fix argument validation in crypto.timingSafeEqual fast path
A regression introduced by 0136bb0 made it possible for the fast path to be hit with non-array-buffer arguments despite that the fast paths could only deal with array buffer arguments, so that it can crash with invalid arguments once crypto.timingSafeEqual is optimized instead of throwing validation errors as usual. This adds validation to the fast path so that it throws correctly. PR-URL: #60538 Fixes: #60537 Refs: nodejs-private/node-private#749 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 3bf42a5 commit ad376c3
Copy full SHA for ad376c3

File tree

Expand file treeCollapse file tree

3 files changed

+55
-26
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+55
-26
lines changed
Open diff view settings
Collapse file

‎src/crypto/crypto_timing.cc‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_timing.cc
+14Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,20 @@ bool FastTimingSafeEqual(Local<Value> receiver,
5656
// NOLINTNEXTLINE(runtime/references)
5757
FastApiCallbackOptions& options) {
5858
HandleScope scope(options.isolate);
59+
if (!IsAnyBufferSource(a_obj)) {
60+
TRACK_V8_FAST_API_CALL("crypto.timingSafeEqual.error");
61+
THROW_ERR_INVALID_ARG_TYPE(options.isolate,
62+
"The \"buf1\" argument must be an instance of "
63+
"ArrayBuffer, Buffer, TypedArray, or DataView.");
64+
return false;
65+
}
66+
if (!IsAnyBufferSource(b_obj)) {
67+
TRACK_V8_FAST_API_CALL("crypto.timingSafeEqual.error");
68+
THROW_ERR_INVALID_ARG_TYPE(options.isolate,
69+
"The \"buf2\" argument must be an instance of "
70+
"ArrayBuffer, Buffer, TypedArray, or DataView.");
71+
return false;
72+
}
5973
ArrayBufferViewContents<uint8_t> a(a_obj);
6074
ArrayBufferViewContents<uint8_t> b(b_obj);
6175
if (a.length() != b.length()) {
Collapse file

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

Copy file name to clipboard
+40Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Test v8 fast path for crypto.timingSafeEqual works correctly.
2+
// Flags: --expose-internals --allow-natives-syntax
3+
'use strict';
4+
5+
const common = require('../common');
6+
7+
if (!common.hasCrypto)
8+
common.skip('missing crypto');
9+
10+
const assert = require('assert');
11+
const crypto = require('crypto');
12+
13+
// V8 Fast API
14+
const foo = Buffer.from('foo');
15+
const bar = Buffer.from('bar');
16+
const longer = Buffer.from('longer');
17+
function testFastPath(buf1, buf2) {
18+
return crypto.timingSafeEqual(buf1, buf2);
19+
}
20+
eval('%PrepareFunctionForOptimization(testFastPath)');
21+
assert.strictEqual(testFastPath(foo, bar), false);
22+
eval('%OptimizeFunctionOnNextCall(testFastPath)');
23+
assert.strictEqual(testFastPath(foo, bar), false);
24+
assert.strictEqual(testFastPath(foo, foo), true);
25+
assert.throws(() => testFastPath(foo, longer), {
26+
code: 'ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH',
27+
});
28+
assert.throws(() => testFastPath(foo, ''), {
29+
code: 'ERR_INVALID_ARG_TYPE',
30+
});
31+
assert.throws(() => testFastPath('', ''), {
32+
code: 'ERR_INVALID_ARG_TYPE',
33+
});
34+
35+
if (common.isDebug) {
36+
const { internalBinding } = require('internal/test/binding');
37+
const { getV8FastApiCallCount } = internalBinding('debug');
38+
assert.strictEqual(getV8FastApiCallCount('crypto.timingSafeEqual.ok'), 2);
39+
assert.strictEqual(getV8FastApiCallCount('crypto.timingSafeEqual.error'), 3);
40+
}
Collapse file

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

Copy file name to clipboardExpand all lines: test/sequential/test-crypto-timing-safe-equal.js
+1-26Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --expose-internals --no-warnings --allow-natives-syntax
1+
// Flags: --no-warnings
22
'use strict';
33
const common = require('../common');
44
if (!common.hasCrypto)
@@ -92,28 +92,3 @@ assert.throws(
9292
name: 'TypeError',
9393
}
9494
);
95-
96-
{
97-
// V8 Fast API
98-
const foo = Buffer.from('foo');
99-
const bar = Buffer.from('bar');
100-
const longer = Buffer.from('longer');
101-
function testFastPath(buf1, buf2) {
102-
return crypto.timingSafeEqual(buf1, buf2);
103-
}
104-
eval('%PrepareFunctionForOptimization(testFastPath)');
105-
assert.strictEqual(testFastPath(foo, bar), false);
106-
eval('%OptimizeFunctionOnNextCall(testFastPath)');
107-
assert.strictEqual(testFastPath(foo, bar), false);
108-
assert.strictEqual(testFastPath(foo, foo), true);
109-
assert.throws(() => testFastPath(foo, longer), {
110-
code: 'ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH',
111-
});
112-
113-
if (common.isDebug) {
114-
const { internalBinding } = require('internal/test/binding');
115-
const { getV8FastApiCallCount } = internalBinding('debug');
116-
assert.strictEqual(getV8FastApiCallCount('crypto.timingSafeEqual.ok'), 2);
117-
assert.strictEqual(getV8FastApiCallCount('crypto.timingSafeEqual.error'), 1);
118-
}
119-
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.