Skip to content

Commit c26b9af

Browse files
thefourtheyeevanlucas
authored andcommitted
tls: copy the Buffer object before using
`convertNPNProtocols` and `convertALPNProtocols' 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. PR-URL: #8055 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
1 parent 8f9fb81 commit c26b9af

File tree

2 files changed

+25
-10
lines changed

2 files changed

+25
-10
lines changed

lib/tls.js

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,22 +49,19 @@ function convertProtocols(protocols) {
4949
exports.convertNPNProtocols = function(protocols, out) {
5050
// If protocols is Array - translate it into buffer
5151
if (Array.isArray(protocols)) {
52-
protocols = convertProtocols(protocols);
53-
}
54-
// If it's already a Buffer - store it
55-
if (protocols instanceof Buffer) {
56-
out.NPNProtocols = protocols;
52+
out.NPNProtocols = convertProtocols(protocols);
53+
} else if (protocols instanceof Buffer) {
54+
// Copy new buffer not to be modified by user.
55+
out.NPNProtocols = Buffer.from(protocols);
5756
}
5857
};
5958

6059
exports.convertALPNProtocols = function(protocols, out) {
6160
// If protocols is Array - translate it into buffer
6261
if (Array.isArray(protocols)) {
63-
protocols = convertProtocols(protocols);
64-
}
65-
// If it's already a Buffer - store it
66-
if (protocols instanceof Buffer) {
67-
// copy new buffer not to be modified by user
62+
out.ALPNProtocols = convertProtocols(protocols);
63+
} else if (protocols instanceof Buffer) {
64+
// Copy new buffer not to be modified by user.
6865
out.ALPNProtocols = Buffer.from(protocols);
6966
}
7067
};

test/parallel/test-tls-basic-validations.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,21 @@ assert.throws(() => tls.createServer({ticketKeys: new Buffer(0)}),
3333

3434
assert.throws(() => tls.createSecurePair({}),
3535
/Error: First argument must be a tls module SecureContext/);
36+
37+
{
38+
const buffer = Buffer.from('abcd');
39+
const out = {};
40+
tls.convertALPNProtocols(buffer, out);
41+
out.ALPNProtocols.write('efgh');
42+
assert(buffer.equals(Buffer.from('abcd')));
43+
assert(out.ALPNProtocols.equals(Buffer.from('efgh')));
44+
}
45+
46+
{
47+
const buffer = Buffer.from('abcd');
48+
const out = {};
49+
tls.convertNPNProtocols(buffer, out);
50+
out.NPNProtocols.write('efgh');
51+
assert(buffer.equals(Buffer.from('abcd')));
52+
assert(out.NPNProtocols.equals(Buffer.from('efgh')));
53+
}

0 commit comments

Comments
 (0)