From c4fe46608acd61fbf7397eadc47378903f95b78a Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Wed, 8 Nov 2017 12:13:20 +0100 Subject: [PATCH] [security] Fix DoS vulnerability Ignore extension and parameter names that are property names of `Object.prototype` when parsing the `Sec-WebSocket-Extensions` header. --- lib/Extensions.js | 17 ++++++++++++++--- test/Extensions.test.js | 21 ++++++++++++++------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/lib/Extensions.js b/lib/Extensions.js index a91910eb2..9cb555494 100644 --- a/lib/Extensions.js +++ b/lib/Extensions.js @@ -15,7 +15,13 @@ const parse = (value) => { value.split(',').forEach((v) => { const params = v.split(';'); const token = params.shift().trim(); - const paramsList = extensions[token] = extensions[token] || []; + + if (extensions[token] === undefined) { + extensions[token] = []; + } else if (!extensions.hasOwnProperty(token)) { + return; + } + const parsedParams = {}; params.forEach((param) => { @@ -34,10 +40,15 @@ const parse = (value) => { value = value.slice(0, value.length - 1); } } - (parsedParams[key] = parsedParams[key] || []).push(value); + + if (parsedParams[key] === undefined) { + parsedParams[key] = [value]; + } else if (parsedParams.hasOwnProperty(key)) { + parsedParams[key].push(value); + } }); - paramsList.push(parsedParams); + extensions[token].push(parsedParams); }); return extensions; diff --git a/test/Extensions.test.js b/test/Extensions.test.js index 8b8fe2fe9..169f68ec5 100644 --- a/test/Extensions.test.js +++ b/test/Extensions.test.js @@ -6,13 +6,13 @@ const Extensions = require('../lib/Extensions'); describe('Extensions', function () { describe('parse', function () { - it('should parse', function () { + it('parses a single extension', function () { const extensions = Extensions.parse('foo'); assert.deepStrictEqual(extensions, { foo: [{}] }); }); - it('should parse params', function () { + it('parses params', function () { const extensions = Extensions.parse('foo; bar; baz=1; bar=2'); assert.deepStrictEqual(extensions, { @@ -20,7 +20,7 @@ describe('Extensions', function () { }); }); - it('should parse multiple extensions', function () { + it('parse multiple extensions', function () { const extensions = Extensions.parse('foo, bar; baz, foo; baz'); assert.deepStrictEqual(extensions, { @@ -29,29 +29,36 @@ describe('Extensions', function () { }); }); - it('should parse quoted params', function () { + it('parses quoted params', function () { const extensions = Extensions.parse('foo; bar="hi"'); assert.deepStrictEqual(extensions, { foo: [{ bar: ['hi'] }] }); }); + + it('ignores names that match Object.prototype properties', function () { + const parse = Extensions.parse; + + assert.deepStrictEqual(parse('hasOwnProperty, toString'), {}); + assert.deepStrictEqual(parse('foo; constructor'), { foo: [{}] }); + }); }); describe('format', function () { - it('should format', function () { + it('formats a single extension', function () { const extensions = Extensions.format({ foo: {} }); assert.strictEqual(extensions, 'foo'); }); - it('should format params', function () { + it('formats params', function () { const extensions = Extensions.format({ foo: { bar: [true, 2], baz: 1 } }); assert.strictEqual(extensions, 'foo; bar; bar=2; baz=1'); }); - it('should format multiple extensions', function () { + it('formats multiple extensions', function () { const extensions = Extensions.format({ foo: [{}, { baz: true }], bar: { baz: true }