From 2ea085595a946bc91a8eed426d25773429b623f1 Mon Sep 17 00:00:00 2001 From: Sakthipriyan Vairamani Date: Thu, 11 Aug 2016 00:46:06 +0530 Subject: [PATCH 1/2] tls: copy the Buffer object before using `convertNPNProtocols` uses the `protocols` buffer object as it is, and if it is modified outside of core, it might have an impact. This patch makes a copy of the buffer object, before using it. --- lib/tls.js | 3 ++- test/parallel/test-tls-basic-validations.js | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/tls.js b/lib/tls.js index 849daeb07f3456..72ee81b7aae3bb 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -53,7 +53,8 @@ exports.convertNPNProtocols = function(protocols, out) { } // If it's already a Buffer - store it if (protocols instanceof Buffer) { - out.NPNProtocols = protocols; + // copy new buffer not to be modified by user + out.NPNProtocols = Buffer.from(protocols); } }; diff --git a/test/parallel/test-tls-basic-validations.js b/test/parallel/test-tls-basic-validations.js index fa0165b3706225..3454da571df458 100644 --- a/test/parallel/test-tls-basic-validations.js +++ b/test/parallel/test-tls-basic-validations.js @@ -38,3 +38,21 @@ assert.throws(() => tls.createServer({ticketKeys: new Buffer(0)}), assert.throws(() => tls.createSecurePair({}), /Error: First argument must be a tls module SecureContext/); + +{ + const buffer = Buffer.from('abcd'); + const out = {}; + tls.convertALPNProtocols(buffer, out); + out.ALPNProtocols.write('efgh'); + assert(buffer.equals(Buffer.from('abcd'))); + assert(out.ALPNProtocols.equals(Buffer.from('efgh'))); +} + +{ + const buffer = Buffer.from('abcd'); + const out = {}; + tls.convertNPNProtocols(buffer, out); + out.NPNProtocols.write('efgh'); + assert(buffer.equals(Buffer.from('abcd'))); + assert(out.NPNProtocols.equals(Buffer.from('efgh'))); +} From 2ee392b0ee4e104d661924b09de48aae3c9dee04 Mon Sep 17 00:00:00 2001 From: Sakthipriyan Vairamani Date: Thu, 11 Aug 2016 07:59:02 +0530 Subject: [PATCH 2/2] fix review comments --- lib/tls.js | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/lib/tls.js b/lib/tls.js index 72ee81b7aae3bb..765e1658ea9127 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -49,11 +49,9 @@ function convertProtocols(protocols) { exports.convertNPNProtocols = function(protocols, out) { // If protocols is Array - translate it into buffer if (Array.isArray(protocols)) { - protocols = convertProtocols(protocols); - } - // If it's already a Buffer - store it - if (protocols instanceof Buffer) { - // copy new buffer not to be modified by user + out.NPNProtocols = convertProtocols(protocols); + } else if (protocols instanceof Buffer) { + // Copy new buffer not to be modified by user. out.NPNProtocols = Buffer.from(protocols); } }; @@ -61,11 +59,9 @@ exports.convertNPNProtocols = function(protocols, out) { exports.convertALPNProtocols = function(protocols, out) { // If protocols is Array - translate it into buffer if (Array.isArray(protocols)) { - protocols = convertProtocols(protocols); - } - // If it's already a Buffer - store it - if (protocols instanceof Buffer) { - // copy new buffer not to be modified by user + out.ALPNProtocols = convertProtocols(protocols); + } else if (protocols instanceof Buffer) { + // Copy new buffer not to be modified by user. out.ALPNProtocols = Buffer.from(protocols); } };