From 24524e96f36976883c7c619811320428536bd4d0 Mon Sep 17 00:00:00 2001 From: Eric Hwang Date: Wed, 17 Apr 2024 14:17:35 -0700 Subject: [PATCH] Guard against prototype pollution --- lib/App.js | 2 ++ lib/PageForServer.js | 4 ++-- lib/components.js | 4 ++++ lib/eventmodel.js | 2 ++ lib/parsing/createPathExpression.js | 2 ++ lib/parsing/index.js | 7 +++++++ lib/templates/expressions.js | 2 ++ lib/templates/templates.js | 9 +++++++++ lib/templates/util.js | 14 ++++++++++++++ test/all/ComponentHarness.mocha.js | 2 +- test/dom/bindings.mocha.js | 25 +++++++++++++++++++++++-- 11 files changed, 68 insertions(+), 5 deletions(-) diff --git a/lib/App.js b/lib/App.js index c7d268de..23a4f525 100644 --- a/lib/App.js +++ b/lib/App.js @@ -14,6 +14,7 @@ var derbyTemplates = require('./templates'); var templates = derbyTemplates.templates; var components = require('./components'); var PageBase = require('./Page'); +var checkKeyIsSafe = require('./templates/util').checkKeyIsSafe; module.exports = App; @@ -258,6 +259,7 @@ App.prototype.component = function(name, constructor, isDependency) { // TODO: DRY. This is copy-pasted from ./templates var mapName = viewName.replace(/:index$/, ''); + checkKeyIsSafe(mapName); var currentView = this.views.nameMap[mapName]; var currentConstructor = (currentView && currentView.componentFactory) ? currentView.componentFactory.constructor : diff --git a/lib/PageForServer.js b/lib/PageForServer.js index 20d44b08..26d185c8 100644 --- a/lib/PageForServer.js +++ b/lib/PageForServer.js @@ -81,11 +81,11 @@ function stringifyBundle(bundle) { // TODO: Cleanup; copied from tracks function pageParams(req) { - var params = { + var params = Object.create(null, { url: req.url, body: req.body, query: req.query, - }; + }); for (var key in req.params) { params[key] = req.params[key]; } diff --git a/lib/components.js b/lib/components.js index d2d1113a..7466bdc5 100644 --- a/lib/components.js +++ b/lib/components.js @@ -12,6 +12,7 @@ var derbyTemplates = require('./templates'); var templates = derbyTemplates.templates; var expressions = derbyTemplates.expressions; var Controller = require('./Controller'); +var checkKeyIsSafe = require('./templates/util').checkKeyIsSafe; var slice = [].slice; exports.Component = Component; @@ -289,15 +290,18 @@ Component.prototype.getAttribute = function(key) { }; Component.prototype.setAttribute = function(key, value) { + checkKeyIsSafe(key); this.context.parent.attributes[key] = value; }; Component.prototype.setNullAttribute = function(key, value) { + checkKeyIsSafe(key); var attributes = this.context.parent.attributes; if (attributes[key] == null) attributes[key] = value; }; function ComponentAttribute(expression, model, key) { + checkKeyIsSafe(key); this.expression = expression; this.model = model; this.key = key; diff --git a/lib/eventmodel.js b/lib/eventmodel.js index c9032b2c..ca88ccf0 100644 --- a/lib/eventmodel.js +++ b/lib/eventmodel.js @@ -1,4 +1,5 @@ var expressions = require('./templates').expressions; +var checkKeyIsSafe = require('./templates/util').checkKeyIsSafe; // The many trees of bindings: // @@ -183,6 +184,7 @@ EventModel.prototype.child = function(segment) { segment = segment.item; } + checkKeyIsSafe(segment); return container[segment] || (container[segment] = new EventModel()); }; diff --git a/lib/parsing/createPathExpression.js b/lib/parsing/createPathExpression.js index c11968d5..8070b383 100644 --- a/lib/parsing/createPathExpression.js +++ b/lib/parsing/createPathExpression.js @@ -1,4 +1,5 @@ var derbyTemplates = require('../templates'); +var checkKeyIsSafe = require('../templates/util').checkKeyIsSafe; var expressions = derbyTemplates.expressions; var operatorFns = derbyTemplates.operatorFns; var esprima = require('esprima-derby'); @@ -217,6 +218,7 @@ function reduceObjectExpression(node) { var property = node.properties[i]; var key = getKeyName(property.key); var expression = reduce(property.value); + checkKeyIsSafe(key); properties[key] = expression; if (isLiteral && expression instanceof expressions.LiteralExpression) { literal[key] = expression.value; diff --git a/lib/parsing/index.js b/lib/parsing/index.js index 16db0bc5..7d5d32ec 100644 --- a/lib/parsing/index.js +++ b/lib/parsing/index.js @@ -1,4 +1,5 @@ var derbyTemplates = require('../templates'); +var checkKeyIsSafe = require('../templates/util').checkKeyIsSafe; var htmlUtil = require('html-util'); var path = require('path'); var App = require('../App'); @@ -111,6 +112,7 @@ function parseHtmlStart(tag, tagName, attributes, selfClosing) { function parseAttributes(attributes) { var attributesMap; for (var key in attributes) { + checkKeyIsSafe(key); if (!attributesMap) attributesMap = {}; var value = attributes[key]; @@ -372,6 +374,7 @@ function attributeValueFromContent(content, isWithin) { function viewAttributesFromElement(element) { var viewAttributes = {}; for (var key in element.attributes) { + checkKeyIsSafe(key); var attribute = element.attributes[key]; var camelCased = dashToCamelCase(key); viewAttributes[camelCased] = @@ -543,6 +546,7 @@ function parseNameAttribute(element) { function parseAttributeElement(element, name, viewAttributes) { var camelName = dashToCamelCase(name); + checkKeyIsSafe(camelName); var isWithin = element.attributes && element.attributes.within; viewAttributes[camelName] = attributeValueFromContent(element.content, isWithin); } @@ -552,6 +556,7 @@ function createAttributesExpression(attributes) { var literalAttributes = {}; var isLiteral = true; for (var key in attributes) { + checkKeyIsSafe(key); var attribute = attributes[key]; if (attribute instanceof expressions.Expression) { dynamicAttributes[key] = attribute; @@ -575,6 +580,7 @@ function parseArrayElement(element, name, viewAttributes) { delete attributes.within; var expression = createAttributesExpression(attributes); var camelName = dashToCamelCase(name); + checkKeyIsSafe(camelName); var viewAttribute = viewAttributes[camelName]; // If viewAttribute is already an ArrayExpression, push the expression for @@ -649,6 +655,7 @@ function viewAttributesFromExpression(expression) { var viewAttributes = {}; for (var key in object) { + checkKeyIsSafe(key); var value = object[key]; viewAttributes[key] = (value instanceof expressions.LiteralExpression) ? value.value : diff --git a/lib/templates/expressions.js b/lib/templates/expressions.js index 84492963..b01e46fb 100644 --- a/lib/templates/expressions.js +++ b/lib/templates/expressions.js @@ -97,6 +97,7 @@ function renderArrayProperties(array, context) { function renderObjectProperties(object, context) { var out = {}; for (var key in object) { + util.checkKeyIsSafe(key); out[key] = renderValue(object[key], context); } return out; @@ -468,6 +469,7 @@ ObjectExpression.prototype.serialize = function() { ObjectExpression.prototype.get = function(context) { var object = {}; for (var key in this.properties) { + util.checkKeyIsSafe(key); var value = this.properties[key].get(context); object[key] = value; } diff --git a/lib/templates/templates.js b/lib/templates/templates.js index bc779383..72a7e681 100644 --- a/lib/templates/templates.js +++ b/lib/templates/templates.js @@ -1117,6 +1117,7 @@ function emitRemoved(context, node, ignore) { } } function emitRemovedBinding(context, ignore, node, property) { + util.checkKeyIsSafe(property); var binding = node[property]; if (binding) { node[property] = null; @@ -1167,6 +1168,7 @@ NodeBinding.prototype.type = 'NodeBinding'; function AttributeBindingsMap() {} function AttributeBinding(template, context, element, name) { + util.checkKeyIsSafe(name); this.template = template; this.context = context; this.element = element; @@ -1510,6 +1512,7 @@ Marker.prototype.get = function() { function ViewAttributesMap(source) { var items = source.split(/\s+/); for (var i = 0, len = items.length; i < len; i++) { + util.checkKeyIsSafe(items[i]); this[items[i]] = true; } } @@ -1517,6 +1520,7 @@ function ViewArraysMap(source) { var items = source.split(/\s+/); for (var i = 0, len = items.length; i < len; i++) { var item = items[i].split('/'); + util.checkKeyIsSafe(item[0]); this[item[0]] = item[1] || item[0]; } } @@ -1832,6 +1836,7 @@ Views.prototype.find = function(name, namespace) { }; Views.prototype.register = function(name, source, options) { var mapName = name.replace(/:index$/, ''); + util.checkKeyIsSafe(mapName); var view = this.nameMap[mapName]; if (view) { // Recreate the view if it already exists. We re-apply the constructor @@ -1846,6 +1851,7 @@ Views.prototype.register = function(name, source, options) { this.nameMap[mapName] = view; // TODO: element is deprecated and should be removed with Derby 0.6.0 var tagName = options && (options.tag || options.element); + util.checkKeyIsSafe(tagName); if (tagName) this.tagMap[tagName] = view; return view; }; @@ -1978,6 +1984,7 @@ AsProperty.prototype.serialize = function() { }; AsProperty.prototype.emit = function(context, target) { var node = traverseAndCreate(context.controller, this.segments); + util.checkKeyIsSafe(this.lastSegment); node[this.lastSegment] = target; this.addListeners(target, node, this.lastSegment); }; @@ -2017,8 +2024,10 @@ AsObject.prototype.serialize = function() { }; AsObject.prototype.emit = function(context, target) { var node = traverseAndCreate(context.controller, this.segments); + util.checkKeyIsSafe(this.lastSegment); var object = node[this.lastSegment] || (node[this.lastSegment] = {}); var key = this.keyExpression.get(context); + util.checkKeyIsSafe(key); object[key] = target; this.addListeners(target, object, key); }; diff --git a/lib/templates/util.js b/lib/templates/util.js index fff5ec35..8be62b8b 100644 --- a/lib/templates/util.js +++ b/lib/templates/util.js @@ -1,3 +1,16 @@ +var objectProtoPropNames = Object.create(null); +Object.getOwnPropertyNames(Object.prototype).forEach(function(prop) { + if (prop !== '__proto__') { + objectProtoPropNames[prop] = true; + } +}); +function checkKeyIsSafe(key) { + if (key === '__proto__' || objectProtoPropNames[key]) { + throw new Error('Unsafe key "' + key + '"'); + } +} +exports.checkKeyIsSafe = checkKeyIsSafe; + exports.concat = function(a, b) { if (!a) return b; if (!b) return a; @@ -17,6 +30,7 @@ exports.traverseAndCreate = function(node, segments) { if (!len) return node; for (var i = 0; i < len; i++) { var segment = segments[i]; + checkKeyIsSafe(segment); node = node[segment] || (node[segment] = {}); } return node; diff --git a/test/all/ComponentHarness.mocha.js b/test/all/ComponentHarness.mocha.js index 644f4563..20c48918 100644 --- a/test/all/ComponentHarness.mocha.js +++ b/test/all/ComponentHarness.mocha.js @@ -259,7 +259,7 @@ describe('ComponentHarness', function() { it('gets overridden without error', function() { function ConflictingClown() {} - ConflictingClown.view = {is: 'clown'}; + ConflictingClown.view = {is: 'clown', source: ''}; function Clown() {} Clown.view = { is: 'clown', diff --git a/test/dom/bindings.mocha.js b/test/dom/bindings.mocha.js index 1d3676ca..db46e42e 100644 --- a/test/dom/bindings.mocha.js +++ b/test/dom/bindings.mocha.js @@ -271,8 +271,29 @@ describe('bindings', function() { expect(page.box.myDiv).to.not.equal(initialElement); done(); }); - }) - }) + }); + + ['__proto__', 'constructor'].forEach(function(badKey) { + it(`disallows prototype modification with ${badKey}`, function() { + var harness = runner.createHarness(` + + `); + function Box() {} + Box.view = { + is: 'box', + source:` + +
one
+ ` + }; + var app = harness.app; + app.component(Box); + expect(() => harness.renderDom()).to.throw(`Unsafe key "${badKey}"`); + // Rendering to HTML string should still work, as that doesn't process `as` attributes + expect(harness.renderHtml().html).to.equal('
one
'); + }); + }); + }); function testArray(itemTemplate, itemData) { it('each on path', function() {