Skip to content

Commit 0f7c06e

Browse files
bnoordhuisBridgeAR
bnoordhuis
authored and
BridgeAR
committed
tls: fix object prototype type confusion
Use `Object.create(null)` for dictionary objects so that keys from certificate strings or the authorityInfoAccess field cannot conflict with Object.prototype properties. PR-URL: #14447 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
1 parent 6eeb06f commit 0f7c06e

4 files changed

Lines changed: 37 additions & 15 deletions

File tree

‎lib/_tls_common.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -199,14 +199,11 @@ exports.translatePeerCertificate = function translatePeerCertificate(c) {
199199
if (c.subject != null) c.subject = tls.parseCertString(c.subject);
200200
if (c.infoAccess != null) {
201201
var info = c.infoAccess;
202-
c.infoAccess = {};
202+
c.infoAccess = Object.create(null);
203203

204204
// XXX: More key validation?
205205
info.replace(/([^\n:]*):([^\n]*)(?:\n|$)/g, function(all, key, val) {
206-
if (key === '__proto__')
207-
return;
208-
209-
if (c.infoAccess.hasOwnProperty(key))
206+
if (key in c.infoAccess)
210207
c.infoAccess[key].push(val);
211208
else
212209
c.infoAccess[key] = [val];

‎lib/tls.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) {
231231
// Example:
232232
// C=US\nST=CA\nL=SF\nO=Joyent\nOU=Node.js\nCN=ca1\[email protected]
233233
exports.parseCertString = function parseCertString(s) {
234-
var out = {};
234+
var out = Object.create(null);
235235
var parts = s.split('\n');
236236
for (var i = 0, len = parts.length; i < len; i++) {
237237
var sepIndex = parts[i].indexOf('=');

‎test/parallel/test-tls-parse-cert-string.js‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable no-proto */
12
'use strict';
23
const common = require('../common');
34
if (!common.hasCrypto)
@@ -11,6 +12,7 @@ const tls = require('tls');
1112
'CN=ca1\[email protected]';
1213
const singlesOut = tls.parseCertString(singles);
1314
assert.deepStrictEqual(singlesOut, {
15+
__proto__: null,
1416
C: 'US',
1517
ST: 'CA',
1618
L: 'SF',
@@ -26,6 +28,7 @@ const tls = require('tls');
2628
'CN=*.nodejs.org';
2729
const doublesOut = tls.parseCertString(doubles);
2830
assert.deepStrictEqual(doublesOut, {
31+
__proto__: null,
2932
OU: [ 'Domain Control Validated', 'PositiveSSL Wildcard' ],
3033
CN: '*.nodejs.org'
3134
});
@@ -34,5 +37,13 @@ const tls = require('tls');
3437
{
3538
const invalid = 'fhqwhgads';
3639
const invalidOut = tls.parseCertString(invalid);
37-
assert.deepStrictEqual(invalidOut, {});
40+
assert.deepStrictEqual(invalidOut, { __proto__: null });
41+
}
42+
43+
{
44+
const input = '__proto__=mostly harmless\nhasOwnProperty=not a function';
45+
const expected = Object.create(null);
46+
expected.__proto__ = 'mostly harmless';
47+
expected.hasOwnProperty = 'not a function';
48+
assert.deepStrictEqual(tls.parseCertString(input), expected);
3849
}

‎test/parallel/test-tls-translate-peer-certificate.js‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable no-proto */
12
'use strict';
23
const common = require('../common');
34

@@ -7,8 +8,12 @@ if (!common.hasCrypto)
78
const { strictEqual, deepStrictEqual } = require('assert');
89
const { translatePeerCertificate } = require('_tls_common');
910

10-
const certString = 'A=1\nB=2\nC=3';
11-
const certObject = { A: '1', B: '2', C: '3' };
11+
const certString = '__proto__=42\nA=1\nB=2\nC=3';
12+
const certObject = Object.create(null);
13+
certObject.__proto__ = '42';
14+
certObject.A = '1';
15+
certObject.B = '2';
16+
certObject.C = '3';
1217

1318
strictEqual(translatePeerCertificate(null), null);
1419
strictEqual(translatePeerCertificate(undefined), null);
@@ -19,14 +24,14 @@ strictEqual(translatePeerCertificate(1), 1);
1924
deepStrictEqual(translatePeerCertificate({}), {});
2025

2126
deepStrictEqual(translatePeerCertificate({ issuer: '' }),
22-
{ issuer: {} });
27+
{ issuer: Object.create(null) });
2328
deepStrictEqual(translatePeerCertificate({ issuer: null }),
2429
{ issuer: null });
2530
deepStrictEqual(translatePeerCertificate({ issuer: certString }),
2631
{ issuer: certObject });
2732

2833
deepStrictEqual(translatePeerCertificate({ subject: '' }),
29-
{ subject: {} });
34+
{ subject: Object.create(null) });
3035
deepStrictEqual(translatePeerCertificate({ subject: null }),
3136
{ subject: null });
3237
deepStrictEqual(translatePeerCertificate({ subject: certString }),
@@ -47,9 +52,18 @@ deepStrictEqual(
4752
}
4853

4954
deepStrictEqual(translatePeerCertificate({ infoAccess: '' }),
50-
{ infoAccess: {} });
55+
{ infoAccess: Object.create(null) });
5156
deepStrictEqual(translatePeerCertificate({ infoAccess: null }),
5257
{ infoAccess: null });
53-
deepStrictEqual(
54-
translatePeerCertificate({ infoAccess: 'OCSP - URI:file:///etc/passwd' }),
55-
{ infoAccess: { 'OCSP - URI': ['file:///etc/passwd'] } });
58+
{
59+
const input =
60+
'__proto__:mostly harmless\n' +
61+
'hasOwnProperty:not a function\n' +
62+
'OCSP - URI:file:///etc/passwd\n';
63+
const expected = Object.create(null);
64+
expected.__proto__ = ['mostly harmless'];
65+
expected.hasOwnProperty = ['not a function'];
66+
expected['OCSP - URI'] = ['file:///etc/passwd'];
67+
deepStrictEqual(translatePeerCertificate({ infoAccess: input }),
68+
{ infoAccess: expected });
69+
}

0 commit comments

Comments
 (0)