GeezerVison is showing github.com — crypto: coerce -0 keylen to +0 in pbkdf2 and scrypt · nodejs/node@61826df
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/crypto/pbkdf2.js +2Lines changed: 2 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -88,6 +88,8 @@ function check(password, salt, iterations, keylen, digest) { | 88 | 88 | // to the 31-bit range here (which is plenty). | 89 | 89 | validateInt32(iterations, 'iterations', 1); | 90 | 90 | validateInt32(keylen, 'keylen', 0); | | 91 | + // Coerce -0 to +0. | | 92 | + keylen += 0; | 91 | 93 |
| 92 | 94 | return { password, salt, iterations, keylen, digest }; | 93 | 95 | } | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/crypto/scrypt.js +2Lines changed: 2 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -83,6 +83,8 @@ function check(password, salt, keylen, options) { | 83 | 83 | password = getArrayBufferOrView(password, 'password'); | 84 | 84 | salt = getArrayBufferOrView(salt, 'salt'); | 85 | 85 | validateInt32(keylen, 'keylen', 0); | | 86 | + // Coerce -0 to +0. | | 87 | + keylen += 0; | 86 | 88 |
| 87 | 89 | let { N, r, p, maxmem } = defaults; | 88 | 90 | if (options && options !== defaults) { | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-crypto-pbkdf2.js +29Lines changed: 29 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -110,6 +110,35 @@ for (const iterations of [-1, 0, 2147483648]) { | 110 | 110 | }); | 111 | 111 | }); | 112 | 112 |
| | 113 | +// `-0` keylen must not abort the process via the native binding's | | 114 | +// IsInt32() assertion. Behavior of `keylen=0` itself varies by OpenSSL | | 115 | +// build (bundled returns an empty buffer; some shared OpenSSL builds | | 116 | +// throw); the requirement here is only that `-0` produces the same | | 117 | +// outcome as `+0`. | | 118 | +{ | | 119 | + let posError; | | 120 | + let posResult; | | 121 | + try { | | 122 | + posResult = crypto.pbkdf2Sync('password', 'salt', 1, 0, 'sha256'); | | 123 | + } catch (err) { | | 124 | + posError = err; | | 125 | + } | | 126 | + let negError; | | 127 | + let negResult; | | 128 | + try { | | 129 | + negResult = crypto.pbkdf2Sync('password', 'salt', 1, -0, 'sha256'); | | 130 | + } catch (err) { | | 131 | + negError = err; | | 132 | + } | | 133 | + if (posError !== undefined) { | | 134 | + assert.strictEqual(negError?.message, posError.message); | | 135 | + } else { | | 136 | + assert.deepStrictEqual(negResult, posResult); | | 137 | + } | | 138 | + | | 139 | + crypto.pbkdf2('password', 'salt', 1, -0, 'sha256', common.mustCall()); | | 140 | +} | | 141 | + | 113 | 142 | // Should not get FATAL ERROR with empty password and salt | 114 | 143 | // https://github.com/nodejs/node/issues/8571 | 115 | 144 | crypto.pbkdf2('', '', 1, 32, 'sha256', common.mustSucceed()); | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-crypto-scrypt.js +27Lines changed: 27 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -272,3 +272,30 @@ for (const { args, expected } of badargs) { | 272 | 272 | ['p', 1], ['parallelization', 1], | 273 | 273 | ].forEach((arg) => testParameter(...arg)); | 274 | 274 | } | | 275 | + | | 276 | +// `-0` keylen must not abort the process via the native binding's | | 277 | +// IsInt32() assertion. Assert that `-0` produces the same outcome as | | 278 | +// `+0` (which differs by OpenSSL build). | | 279 | +{ | | 280 | + let posError; | | 281 | + let posResult; | | 282 | + try { | | 283 | + posResult = crypto.scryptSync('', '', 0); | | 284 | + } catch (err) { | | 285 | + posError = err; | | 286 | + } | | 287 | + let negError; | | 288 | + let negResult; | | 289 | + try { | | 290 | + negResult = crypto.scryptSync('', '', -0); | | 291 | + } catch (err) { | | 292 | + negError = err; | | 293 | + } | | 294 | + if (posError !== undefined) { | | 295 | + assert.strictEqual(negError?.message, posError.message); | | 296 | + } else { | | 297 | + assert.deepStrictEqual(negResult, posResult); | | 298 | + } | | 299 | + | | 300 | + crypto.scrypt('', '', -0, common.mustCall()); | | 301 | +} |
|