Browse Source

crypto,tls: fix mutability of return values

If you alter the array returned by `tls.getCiphers()`,
`crypto.getCiphers()`, `crypto.getHashes()`, or `crypto.getCurves()`, it
will alter subsequent return values from those functions.

```js
'use strict';

const crypto = require('crypto');

var hashes = crypto.getHashes();

hashes.splice(0, hashes.length);

hashes.push('some-arbitrary-value');

console.log(crypto.getHashes()); // "['some-arbitrary-value']"
```

This is surprising. Change functions to return copy of array instead.

PR-URL: https://github.com/nodejs/node/pull/10795
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
v6
Rich Trott 8 years ago
parent
commit
56950674d6
  1. 2
      lib/internal/util.js
  2. 6
      lib/tls.js
  3. 14
      test/parallel/test-crypto.js

2
lib/internal/util.js

@ -158,7 +158,7 @@ exports.cachedResult = function cachedResult(fn) {
return () => {
if (result === undefined)
result = fn();
return result;
return result.slice();
};
};

6
lib/tls.js

@ -22,9 +22,9 @@ exports.DEFAULT_CIPHERS =
exports.DEFAULT_ECDH_CURVE = 'prime256v1';
exports.getCiphers = internalUtil.cachedResult(() => {
return internalUtil.filterDuplicateStrings(binding.getSSLCiphers(), true);
});
exports.getCiphers = internalUtil.cachedResult(
() => internalUtil.filterDuplicateStrings(binding.getSSLCiphers(), true)
);
// Convert protocols array into valid OpenSSL protocols list
// ("\x06spdy/2\x08http/1.1\x08http/1.0")

14
test/parallel/test-crypto.js

@ -97,6 +97,20 @@ assert(crypto.getCurves().includes('secp384r1'));
assert(!crypto.getCurves().includes('SECP384R1'));
validateList(crypto.getCurves());
// Modifying return value from get* functions should not mutate subsequent
// return values.
function testImmutability(fn) {
const list = fn();
const copy = [...list];
list.push('some-arbitrary-value');
assert.deepStrictEqual(fn(), copy);
}
testImmutability(crypto.getCiphers);
testImmutability(tls.getCiphers);
testImmutability(crypto.getHashes);
testImmutability(crypto.getCurves);
// Regression tests for #5725: hex input that's not a power of two should
// throw, not assert in C++ land.
assert.throws(function() {

Loading…
Cancel
Save