From ccb6045f2dc226e2c5a0e9c0536200929b498fa3 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Fri, 13 Jan 2017 14:28:35 -0800
Subject: [PATCH] crypto,tls: fix mutability of return values
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 lib/internal/util.js         |  2 +-
 lib/tls.js                   |  6 +++---
 test/parallel/test-crypto.js | 14 ++++++++++++++
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/lib/internal/util.js b/lib/internal/util.js
index d0b6c272aaba0a..535bea967e6d3b 100644
--- a/lib/internal/util.js
+++ b/lib/internal/util.js
@@ -127,7 +127,7 @@ exports.cachedResult = function cachedResult(fn) {
   return () => {
     if (result === undefined)
       result = fn();
-    return result;
+    return result.slice();
   };
 };
 
diff --git a/lib/tls.js b/lib/tls.js
index 32c0319754be2a..56da56029fc94b 100644
--- a/lib/tls.js
+++ b/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")
diff --git a/test/parallel/test-crypto.js b/test/parallel/test-crypto.js
index a3610b2b1708f6..5edf748994ebdd 100644
--- a/test/parallel/test-crypto.js
+++ b/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() {