FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

crypto: coerce -0 to +0 before native calls · nodejs/node@73aba92 · GitHub

/ node Public

Commit 73aba92

Browse files
panva
authored and
aduh95
committed
crypto: coerce -0 to +0 before native calls
Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #63556 Backport-PR-URL: #64629 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent c83b798 commit 73aba92

7 files changed

Lines changed: 213 additions & 14 deletions

File tree

‎lib/internal/crypto/cipher.js‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ function getUIntOption(options, key) {
108108
if (options && (value = options[key]) != null) {
109109
if (value >>> 0 !== value)
110110
throw new ERR_INVALID_ARG_VALUE(`options.${key}`, value);
111-
return value;
111+
// Coerce -0 to +0.
112+
return value + 0;
112113
}
113114
return -1;
114115
}
@@ -265,10 +266,16 @@ function getCipherInfo(nameOrNid, options) {
265266
if (options !== undefined) {
266267
validateObject(options, 'options');
267268
({ keyLength, ivLength } = options);
268-
if (keyLength !== undefined)
269+
if (keyLength !== undefined) {
269270
validateInt32(keyLength, 'options.keyLength');
270-
if (ivLength !== undefined)
271+
// Coerce -0 to +0.
272+
keyLength += 0;
273+
}
274+
if (ivLength !== undefined) {
271275
validateInt32(ivLength, 'options.ivLength');
276+
// Coerce -0 to +0.
277+
ivLength += 0;
278+
}
272279
}
273280

274281
const ret = _getCipherInfo({}, nameOrNid, keyLength, ivLength);

‎lib/internal/crypto/diffiehellman.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,11 @@ function DiffieHellman(sizeOrKey, keyEncoding, generator, genEncoding) {
9898
// rejected with ERR_OSSL_BN_BITS_TOO_SMALL) by OpenSSL. The glue code
9999
// in node_crypto.cc accepts values that are IsInt32() for that reason
100100
// and that's why we do that here too.
101-
if (typeof sizeOrKey === 'number')
101+
if (typeof sizeOrKey === 'number') {
102102
validateInt32(sizeOrKey, 'sizeOrKey');
103+
// Coerce -0 to +0.
104+
sizeOrKey += 0;
105+
}
103106

104107
if (keyEncoding && !Buffer.isEncoding(keyEncoding) &&
105108
keyEncoding !== 'buffer') {

‎lib/internal/crypto/hash.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,13 @@ function Hash(algorithm, options) {
9595
const isCopy = algorithm instanceof _Hash;
9696
if (!isCopy)
9797
validateString(algorithm, 'algorithm');
98-
const xofLen = typeof options === 'object' && options !== null ?
98+
let xofLen = typeof options === 'object' && options !== null ?
9999
options.outputLength : undefined;
100-
if (xofLen !== undefined)
100+
if (xofLen !== undefined) {
101101
validateUint32(xofLen, 'options.outputLength');
102+
// Coerce -0 to +0.
103+
xofLen += 0;
104+
}
102105
// Lookup the cached ID from JS land because it's faster than decoding
103106
// the string in C++ land.
104107
const algorithmId = isCopy ? -1 : getCachedHashId(algorithm);
@@ -288,6 +291,8 @@ function hash(algorithm, input, options) {
288291

289292
if (outputLength !== undefined) {
290293
validateUint32(outputLength, 'outputLength');
294+
// Coerce -0 to +0.
295+
outputLength += 0;
291296
}
292297

293298
if (outputLength === undefined) {

‎lib/internal/crypto/hkdf.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ const validateParameters = hideStackFrames((hash, key, salt, info, length) => {
5858
info = validateByteSource.withoutStackTrace(info, 'info');
5959

6060
validateInteger.withoutStackTrace(length, 'length', 0, kMaxLength);
61+
// Coerce -0 to +0.
62+
length += 0;
6163

6264
if (info.byteLength > 1024) {
6365
throw new ERR_OUT_OF_RANGE.HideStackFramesError(

‎lib/internal/crypto/keygen.js‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -219,14 +219,18 @@ function createJob(mode, type, options) {
219219
case 'rsa-pss':
220220
{
221221
validateObject(options, 'options');
222-
const { modulusLength } = options;
222+
let { modulusLength } = options;
223223
validateUint32(modulusLength, 'options.modulusLength');
224+
// Coerce -0 to +0.
225+
modulusLength += 0;
224226

225227
let { publicExponent } = options;
226228
if (publicExponent == null) {
227229
publicExponent = 0x10001;
228230
} else {
229231
validateUint32(publicExponent, 'options.publicExponent');
232+
// Coerce -0 to +0.
233+
publicExponent += 0;
230234
}
231235

232236
if (type === 'rsa') {
@@ -239,11 +243,15 @@ function createJob(mode, type, options) {
239243
}
240244

241245
const {
242-
hash, mgf1Hash, hashAlgorithm, mgf1HashAlgorithm, saltLength,
246+
hash, mgf1Hash, hashAlgorithm, mgf1HashAlgorithm,
243247
} = options;
248+
let { saltLength } = options;
244249

245-
if (saltLength !== undefined)
250+
if (saltLength !== undefined) {
246251
validateInt32(saltLength, 'options.saltLength', 0);
252+
// Coerce -0 to +0.
253+
saltLength += 0;
254+
}
247255
if (hashAlgorithm !== undefined)
248256
validateString(hashAlgorithm, 'options.hashAlgorithm');
249257
if (mgf1HashAlgorithm !== undefined)
@@ -284,14 +292,19 @@ function createJob(mode, type, options) {
284292
case 'dsa':
285293
{
286294
validateObject(options, 'options');
287-
const { modulusLength } = options;
295+
let { modulusLength } = options;
288296
validateUint32(modulusLength, 'options.modulusLength');
297+
// Coerce -0 to +0.
298+
modulusLength += 0;
289299

290300
let { divisorLength } = options;
291301
if (divisorLength == null) {
292302
divisorLength = -1;
293-
} else
303+
} else {
294304
validateInt32(divisorLength, 'options.divisorLength', 0);
305+
// Coerce -0 to +0.
306+
divisorLength += 0;
307+
}
295308

296309
return new DsaKeyPairGenJob(
297310
mode,
@@ -321,7 +334,8 @@ function createJob(mode, type, options) {
321334
case 'dh':
322335
{
323336
validateObject(options, 'options');
324-
const { group, primeLength, prime, generator } = options;
337+
const { group, prime } = options;
338+
let { primeLength, generator } = options;
325339
if (group != null) {
326340
if (prime != null)
327341
throw new ERR_INCOMPATIBLE_OPTION_PAIR('group', 'prime');
@@ -342,13 +356,17 @@ function createJob(mode, type, options) {
342356
validateBuffer(prime, 'options.prime');
343357
} else if (primeLength != null) {
344358
validateInt32(primeLength, 'options.primeLength', 0);
359+
// Coerce -0 to +0.
360+
primeLength += 0;
345361
} else {
346362
throw new ERR_MISSING_OPTION(
347363
'At least one of the group, prime, or primeLength options');
348364
}
349365

350366
if (generator != null) {
351367
validateInt32(generator, 'options.generator', 0);
368+
// Coerce -0 to +0.
369+
generator += 0;
352370
}
353371
return new DhKeyPairGenJob(
354372
mode,

‎lib/internal/crypto/random.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,12 +603,14 @@ function checkPrime(candidate, options = kEmptyObject, callback) {
603603
}
604604
validateFunction(callback, 'callback');
605605
validateObject(options, 'options');
606-
const {
606+
let {
607607
checks = 0,
608608
} = options;
609609

610610
// The checks option is unsigned but must fit into a signed C int for OpenSSL.
611611
validateInt32(checks, 'options.checks', 0);
612+
// Coerce -0 to +0.
613+
checks += 0;
612614

613615
const job = new CheckPrimeJob(kCryptoJobAsync, candidate, checks);
614616
job.ondone = callback;
@@ -632,12 +634,14 @@ function checkPrimeSync(candidate, options = kEmptyObject) {
632634
);
633635
}
634636
validateObject(options, 'options');
635-
const {
637+
let {
636638
checks = 0,
637639
} = options;
638640

639641
// The checks option is unsigned but must fit into a signed C int for OpenSSL.
640642
validateInt32(checks, 'options.checks', 0);
643+
// Coerce -0 to +0.
644+
checks += 0;
641645

642646
const job = new CheckPrimeJob(kCryptoJobSync, candidate, checks);
643647
const { 0: err, 1: result } = job.run();

‎test/parallel/test-crypto-negative-zero.js‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
const assert = require('assert');
8+
const crypto = require('crypto');
9+
const { hasOpenSSL } = require('../common/crypto');
10+
11+
function getOutcome(fn) {
12+
try {
13+
return { result: fn() };
14+
} catch (err) {
15+
return { err };
16+
}
17+
}
18+
19+
function assertSameOutcome(actual, expected) {
20+
if (expected.err !== undefined) {
21+
assert(actual.err instanceof Error);
22+
assert.strictEqual(actual.err.name, expected.err.name);
23+
assert.strictEqual(actual.err.code, expected.err.code);
24+
assert.strictEqual(actual.err.message, expected.err.message);
25+
} else {
26+
assert.deepStrictEqual(actual.result, expected.result);
27+
}
28+
}
29+
30+
function assertSameErrorOrSuccess(actual, expected) {
31+
if (expected.err !== undefined) {
32+
assert(actual.err instanceof Error);
33+
assert.strictEqual(actual.err.name, expected.err.name);
34+
assert.strictEqual(actual.err.code, expected.err.code);
35+
assert.strictEqual(actual.err.message, expected.err.message);
36+
} else {
37+
assert.strictEqual(actual.err, undefined);
38+
}
39+
}
40+
41+
{
42+
const expected = getOutcome(() =>
43+
crypto.hkdfSync('sha256', 'key', 'salt', 'info', 0),
44+
);
45+
assertSameOutcome(
46+
getOutcome(() => crypto.hkdfSync('sha256', 'key', 'salt', 'info', -0)),
47+
expected,
48+
);
49+
crypto.hkdf('sha256', 'key', 'salt', 'info', -0,
50+
common.mustCall((err, result) => {
51+
assertSameOutcome({ err, result }, expected);
52+
}));
53+
}
54+
55+
{
56+
assert.strictEqual(
57+
crypto.checkPrimeSync(Buffer.from([3]), { checks: -0 }),
58+
true,
59+
);
60+
crypto.checkPrime(Buffer.from([3]), { checks: -0 },
61+
common.mustSucceed((result) => {
62+
assert.strictEqual(result, true);
63+
}));
64+
}
65+
66+
{
67+
assert.throws(() => crypto.createDiffieHellman(-0, 2), {
68+
name: 'Error',
69+
});
70+
}
71+
72+
{
73+
for (const [type, getOptions] of [
74+
['rsa', (zero) => ({ modulusLength: zero })],
75+
['rsa', (zero) => ({ modulusLength: 512, publicExponent: zero })],
76+
['rsa-pss', (zero) => ({
77+
modulusLength: 512,
78+
publicExponent: 65537,
79+
saltLength: zero,
80+
})],
81+
['dsa', (zero) => ({ modulusLength: zero })],
82+
['dh', (zero) => ({ primeLength: zero })],
83+
['dh', (zero) => ({ primeLength: 2, generator: zero })],
84+
]) {
85+
assertSameErrorOrSuccess(
86+
getOutcome(() => crypto.generateKeyPairSync(type, getOptions(-0))),
87+
getOutcome(() => crypto.generateKeyPairSync(type, getOptions(0))),
88+
);
89+
}
90+
91+
if (!hasOpenSSL(3)) {
92+
common.printSkipMessage(
93+
'Skipping DSA divisorLength 0 key generation on OpenSSL 1.1.1');
94+
} else {
95+
assertSameErrorOrSuccess(
96+
getOutcome(() => crypto.generateKeyPairSync('dsa', {
97+
modulusLength: 512,
98+
divisorLength: -0,
99+
})),
100+
getOutcome(() => crypto.generateKeyPairSync('dsa', {
101+
modulusLength: 512,
102+
divisorLength: 0,
103+
})),
104+
);
105+
}
106+
107+
crypto.generateKeyPair('rsa', { modulusLength: -0 },
108+
common.mustCall((err) => {
109+
assert(err instanceof Error);
110+
}));
111+
}
112+
113+
if (!process.features.openssl_is_boringssl) {
114+
assert.strictEqual(
115+
crypto.createHash('shake128', { outputLength: -0 }).digest('hex'),
116+
'',
117+
);
118+
assert.strictEqual(
119+
crypto.createHash('shake128', { outputLength: 5 })
120+
.copy({ outputLength: -0 })
121+
.digest('hex'),
122+
'',
123+
);
124+
assert.strictEqual(
125+
crypto.hash('shake128', 'data', { outputLength: -0 }),
126+
'',
127+
);
128+
}
129+
130+
{
131+
const key = Buffer.alloc(16);
132+
const iv = Buffer.alloc(12);
133+
134+
assertSameErrorOrSuccess(
135+
getOutcome(() => crypto.createCipheriv(
136+
'aes-128-gcm', key, iv, { authTagLength: -0 })),
137+
getOutcome(() => crypto.createCipheriv(
138+
'aes-128-gcm', key, iv, { authTagLength: 0 })),
139+
);
140+
assertSameErrorOrSuccess(
141+
getOutcome(() => crypto.createCipheriv(
142+
'aes-128-gcm', key, iv).setAAD(
143+
Buffer.alloc(0),
144+
{ plaintextLength: -0 },
145+
)),
146+
getOutcome(() => crypto.createCipheriv(
147+
'aes-128-gcm', key, iv).setAAD(
148+
Buffer.alloc(0),
149+
{ plaintextLength: 0 },
150+
)),
151+
);
152+
assert.strictEqual(
153+
crypto.getCipherInfo('aes-128-cbc', { keyLength: -0 }),
154+
undefined,
155+
);
156+
assert.strictEqual(
157+
crypto.getCipherInfo('aes-128-cbc', { ivLength: -0 }),
158+
undefined,
159+
);
160+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL