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: https://github.com/nodejs/node/pull/14447
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This commit is contained in:
Ben Noordhuis 2017-07-24 14:36:36 +02:00 committed by Ruben Bridgewater
parent 6eeb06f234
commit 0f7c06eb2d
No known key found for this signature in database
GPG Key ID: F07496B3EB3C1762
4 changed files with 37 additions and 15 deletions

View File

@ -199,14 +199,11 @@ exports.translatePeerCertificate = function translatePeerCertificate(c) {
if (c.subject != null) c.subject = tls.parseCertString(c.subject);
if (c.infoAccess != null) {
var info = c.infoAccess;
c.infoAccess = {};
c.infoAccess = Object.create(null);
// XXX: More key validation?
info.replace(/([^\n:]*):([^\n]*)(?:\n|$)/g, function(all, key, val) {
if (key === '__proto__')
return;
if (c.infoAccess.hasOwnProperty(key))
if (key in c.infoAccess)
c.infoAccess[key].push(val);
else
c.infoAccess[key] = [val];

View File

@ -231,7 +231,7 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) {
// Example:
// C=US\nST=CA\nL=SF\nO=Joyent\nOU=Node.js\nCN=ca1\nemailAddress=ry@clouds.org
exports.parseCertString = function parseCertString(s) {
var out = {};
var out = Object.create(null);
var parts = s.split('\n');
for (var i = 0, len = parts.length; i < len; i++) {
var sepIndex = parts[i].indexOf('=');

View File

@ -1,3 +1,4 @@
/* eslint-disable no-proto */
'use strict';
const common = require('../common');
if (!common.hasCrypto)
@ -11,6 +12,7 @@ const tls = require('tls');
'CN=ca1\nemailAddress=ry@clouds.org';
const singlesOut = tls.parseCertString(singles);
assert.deepStrictEqual(singlesOut, {
__proto__: null,
C: 'US',
ST: 'CA',
L: 'SF',
@ -26,6 +28,7 @@ const tls = require('tls');
'CN=*.nodejs.org';
const doublesOut = tls.parseCertString(doubles);
assert.deepStrictEqual(doublesOut, {
__proto__: null,
OU: [ 'Domain Control Validated', 'PositiveSSL Wildcard' ],
CN: '*.nodejs.org'
});
@ -34,5 +37,13 @@ const tls = require('tls');
{
const invalid = 'fhqwhgads';
const invalidOut = tls.parseCertString(invalid);
assert.deepStrictEqual(invalidOut, {});
assert.deepStrictEqual(invalidOut, { __proto__: null });
}
{
const input = '__proto__=mostly harmless\nhasOwnProperty=not a function';
const expected = Object.create(null);
expected.__proto__ = 'mostly harmless';
expected.hasOwnProperty = 'not a function';
assert.deepStrictEqual(tls.parseCertString(input), expected);
}

View File

@ -1,3 +1,4 @@
/* eslint-disable no-proto */
'use strict';
const common = require('../common');
@ -7,8 +8,12 @@ if (!common.hasCrypto)
const { strictEqual, deepStrictEqual } = require('assert');
const { translatePeerCertificate } = require('_tls_common');
const certString = 'A=1\nB=2\nC=3';
const certObject = { A: '1', B: '2', C: '3' };
const certString = '__proto__=42\nA=1\nB=2\nC=3';
const certObject = Object.create(null);
certObject.__proto__ = '42';
certObject.A = '1';
certObject.B = '2';
certObject.C = '3';
strictEqual(translatePeerCertificate(null), null);
strictEqual(translatePeerCertificate(undefined), null);
@ -19,14 +24,14 @@ strictEqual(translatePeerCertificate(1), 1);
deepStrictEqual(translatePeerCertificate({}), {});
deepStrictEqual(translatePeerCertificate({ issuer: '' }),
{ issuer: {} });
{ issuer: Object.create(null) });
deepStrictEqual(translatePeerCertificate({ issuer: null }),
{ issuer: null });
deepStrictEqual(translatePeerCertificate({ issuer: certString }),
{ issuer: certObject });
deepStrictEqual(translatePeerCertificate({ subject: '' }),
{ subject: {} });
{ subject: Object.create(null) });
deepStrictEqual(translatePeerCertificate({ subject: null }),
{ subject: null });
deepStrictEqual(translatePeerCertificate({ subject: certString }),
@ -47,9 +52,18 @@ deepStrictEqual(
}
deepStrictEqual(translatePeerCertificate({ infoAccess: '' }),
{ infoAccess: {} });
{ infoAccess: Object.create(null) });
deepStrictEqual(translatePeerCertificate({ infoAccess: null }),
{ infoAccess: null });
deepStrictEqual(
translatePeerCertificate({ infoAccess: 'OCSP - URI:file:///etc/passwd' }),
{ infoAccess: { 'OCSP - URI': ['file:///etc/passwd'] } });
{
const input =
'__proto__:mostly harmless\n' +
'hasOwnProperty:not a function\n' +
'OCSP - URI:file:///etc/passwd\n';
const expected = Object.create(null);
expected.__proto__ = ['mostly harmless'];
expected.hasOwnProperty = ['not a function'];
expected['OCSP - URI'] = ['file:///etc/passwd'];
deepStrictEqual(translatePeerCertificate({ infoAccess: input }),
{ infoAccess: expected });
}