Skip to content

Commit 9984b05

Browse files
panvaaduh95
authored andcommitted
crypto: coerce -0 to +0 before native calls
Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #63556 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent 7d9fdda commit 9984b05

7 files changed

Lines changed: 210 additions & 15 deletions

File tree

‎lib/internal/crypto/cipher.js‎

Lines changed: 7 additions & 2 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
thrownewERR_INVALID_ARG_VALUE(`options.${key}`,value);
111-
returnvalue;
111+
// Coerce -0 to +0.
112+
returnvalue+0;
112113
}
113114
return-1;
114115
}
@@ -256,12 +257,16 @@ const kMinNid = 1;
256257
constkMaxNid=2_147_483_647;
257258
functiongetCipherInfo(nameOrNid,options={}){
258259
validateObject(options,'options');
259-
const{ keyLength, ivLength }=options;
260+
let{ keyLength, ivLength }=options;
260261
if(keyLength!==undefined){
261262
validateUint32(keyLength,'options.keyLength');
263+
// Coerce -0 to +0.
264+
keyLength+=0;
262265
}
263266
if(ivLength!==undefined){
264267
validateUint32(ivLength,'options.ivLength');
268+
// Coerce -0 to +0.
269+
ivLength+=0;
265270
}
266271

267272
consttype=typeofnameOrNid;

‎lib/internal/crypto/diffiehellman.js‎

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

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

‎lib/internal/crypto/hash.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,13 @@ function Hash(algorithm, options) {
9393
constisCopy=algorithminstanceof_Hash;
9494
if(!isCopy)
9595
validateString(algorithm,'algorithm');
96-
constxofLen=typeofoptions==='object'&&options!==null ?
96+
letxofLen=typeofoptions==='object'&&options!==null ?
9797
options.outputLength : undefined;
98-
if(xofLen!==undefined)
98+
if(xofLen!==undefined){
9999
validateUint32(xofLen,'options.outputLength');
100+
// Coerce -0 to +0.
101+
xofLen+=0;
102+
}
100103
// Lookup the cached ID from JS land because it's faster than decoding
101104
// the string in C++ land.
102105
constalgorithmId=isCopy ? -1 : getCachedHashId(algorithm);
@@ -285,6 +288,8 @@ function hash(algorithm, input, options) {
285288

286289
if(outputLength!==undefined){
287290
validateUint32(outputLength,'outputLength');
291+
// Coerce -0 to +0.
292+
outputLength+=0;
288293
}
289294

290295
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
thrownewERR_OUT_OF_RANGE.HideStackFramesError(

‎lib/internal/crypto/keygen.js‎

Lines changed: 24 additions & 8 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'){
@@ -238,12 +242,14 @@ function createJob(mode, type, options) {
238242
...encoding);
239243
}
240244

241-
const{
242-
hashAlgorithm, mgf1HashAlgorithm, saltLength,
243-
}=options;
245+
const{ hashAlgorithm, mgf1HashAlgorithm }=options;
246+
let{ saltLength }=options;
244247

245-
if(saltLength!==undefined)
248+
if(saltLength!==undefined){
246249
validateInt32(saltLength,'options.saltLength',0);
250+
// Coerce -0 to +0.
251+
saltLength+=0;
252+
}
247253
if(hashAlgorithm!==undefined)
248254
validateString(hashAlgorithm,'options.hashAlgorithm');
249255
if(mgf1HashAlgorithm!==undefined)
@@ -284,14 +290,19 @@ function createJob(mode, type, options) {
284290
case'dsa':
285291
{
286292
validateObject(options,'options');
287-
const{ modulusLength }=options;
293+
let{ modulusLength }=options;
288294
validateUint32(modulusLength,'options.modulusLength');
295+
// Coerce -0 to +0.
296+
modulusLength+=0;
289297

290298
let{ divisorLength }=options;
291299
if(divisorLength==null){
292300
divisorLength=-1;
293-
}else
301+
}else{
294302
validateInt32(divisorLength,'options.divisorLength',0);
303+
// Coerce -0 to +0.
304+
divisorLength+=0;
305+
}
295306

296307
returnnewDsaKeyPairGenJob(
297308
mode,
@@ -321,7 +332,8 @@ function createJob(mode, type, options) {
321332
case'dh':
322333
{
323334
validateObject(options,'options');
324-
const{ group, primeLength, prime, generator }=options;
335+
const{ group, prime }=options;
336+
let{ primeLength, generator }=options;
325337
if(group!=null){
326338
if(prime!=null)
327339
thrownewERR_INCOMPATIBLE_OPTION_PAIR('group','prime');
@@ -342,13 +354,17 @@ function createJob(mode, type, options) {
342354
validateBuffer(prime,'options.prime');
343355
}elseif(primeLength!=null){
344356
validateInt32(primeLength,'options.primeLength',0);
357+
// Coerce -0 to +0.
358+
primeLength+=0;
345359
}else{
346360
thrownewERR_MISSING_OPTION(
347361
'At least one of the group, prime, or primeLength options');
348362
}
349363

350364
if(generator!=null){
351365
validateInt32(generator,'options.generator',0);
366+
// Coerce -0 to +0.
367+
generator+=0;
352368
}
353369
returnnewDhKeyPairGenJob(
354370
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
constjob=newCheckPrimeJob(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
constjob=newCheckPrimeJob(kCryptoJobSync,candidate,checks);
643647
const{0: err,1: result}=job.run();
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
'use strict';
2+
3+
constcommon=require('../common');
4+
if(!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
constassert=require('assert');
8+
constcrypto=require('crypto');
9+
const{ hasOpenSSL }=require('../common/crypto');
10+
11+
functiongetOutcome(fn){
12+
try{
13+
return{result: fn()};
14+
}catch(err){
15+
return{ err };
16+
}
17+
}
18+
19+
functionassertSameOutcome(actual,expected){
20+
if(expected.err!==undefined){
21+
assert(actual.errinstanceofError);
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+
functionassertSameErrorOrSuccess(actual,expected){
31+
if(expected.err!==undefined){
32+
assert(actual.errinstanceofError);
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+
constexpected=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(errinstanceofError);
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+
constkey=Buffer.alloc(16);
132+
constiv=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)