Commit ccb6045
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: #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>1 parent e4e9f67 commit ccb6045
3 files changed
Lines changed: 18 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -127,7 +127,7 @@ exports.cachedResult = function cachedResult(fn) { | |||
| 127 | 127 | return () => { | |
| 128 | 128 | if (result === undefined) | |
| 129 | 129 | result = fn(); | |
| 130 | - return result; | ||
| 130 | + return result.slice(); | ||
| 131 | 131 | }; | |
| 132 | 132 | }; | |
| 133 | 133 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -22,9 +22,9 @@ exports.DEFAULT_CIPHERS = | |||
| 22 | 22 | ||
| 23 | 23 | exports.DEFAULT_ECDH_CURVE = 'prime256v1'; | |
| 24 | 24 | ||
| 25 | - exports.getCiphers = internalUtil.cachedResult(() => { | ||
| 26 | - return internalUtil.filterDuplicateStrings(binding.getSSLCiphers(), true); | ||
| 27 | - }); | ||
| 25 | + exports.getCiphers = internalUtil.cachedResult( | ||
| 26 | + () => internalUtil.filterDuplicateStrings(binding.getSSLCiphers(), true) | ||
| 27 | + ); | ||
| 28 | 28 | ||
| 29 | 29 | // Convert protocols array into valid OpenSSL protocols list | |
| 30 | 30 | // ("\x06spdy/2\x08http/1.1\x08http/1.0") | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -97,6 +97,20 @@ assert(crypto.getCurves().includes('secp384r1')); | |||
| 97 | 97 | assert(!crypto.getCurves().includes('SECP384R1')); | |
| 98 | 98 | validateList(crypto.getCurves()); | |
| 99 | 99 | ||
| 100 | + // Modifying return value from get* functions should not mutate subsequent | ||
| 101 | + // return values. | ||
| 102 | + function testImmutability(fn) { | ||
| 103 | + const list = fn(); | ||
| 104 | + const copy = [...list]; | ||
| 105 | + list.push('some-arbitrary-value'); | ||
| 106 | + assert.deepStrictEqual(fn(), copy); | ||
| 107 | + } | ||
| 108 | + | ||
| 109 | + testImmutability(crypto.getCiphers); | ||
| 110 | + testImmutability(tls.getCiphers); | ||
| 111 | + testImmutability(crypto.getHashes); | ||
| 112 | + testImmutability(crypto.getCurves); | ||
| 113 | + | ||
| 100 | 114 | // Regression tests for #5725: hex input that's not a power of two should | |
| 101 | 115 | // throw, not assert in C++ land. | |
| 102 | 116 | assert.throws(function() { | |
0 commit comments