Skip to content

Commit

Permalink
Guard against prototype pollution
Browse files Browse the repository at this point in the history
  • Loading branch information
ericyhwang committed Apr 17, 2024
1 parent 3d433bb commit 24524e9
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 5 deletions.
2 changes: 2 additions & 0 deletions lib/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 :
Expand Down
4 changes: 2 additions & 2 deletions lib/PageForServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
4 changes: 4 additions & 0 deletions lib/components.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions lib/eventmodel.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
var expressions = require('./templates').expressions;
var checkKeyIsSafe = require('./templates/util').checkKeyIsSafe;

// The many trees of bindings:
//
Expand Down Expand Up @@ -183,6 +184,7 @@ EventModel.prototype.child = function(segment) {
segment = segment.item;
}

checkKeyIsSafe(segment);
return container[segment] || (container[segment] = new EventModel());
};

Expand Down
2 changes: 2 additions & 0 deletions lib/parsing/createPathExpression.js
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions lib/parsing/index.js
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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] =
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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 :
Expand Down
2 changes: 2 additions & 0 deletions lib/templates/expressions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
9 changes: 9 additions & 0 deletions lib/templates/templates.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1510,13 +1512,15 @@ 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;
}
}
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];
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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;
};
Expand Down Expand Up @@ -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);
};
Expand Down Expand Up @@ -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);
};
Expand Down
14 changes: 14 additions & 0 deletions lib/templates/util.js
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion test/all/ComponentHarness.mocha.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ describe('ComponentHarness', function() {

it('gets overridden without error', function() {
function ConflictingClown() {}
ConflictingClown.view = {is: 'clown'};
ConflictingClown.view = {is: 'clown', source: '<index:>'};
function Clown() {}
Clown.view = {
is: 'clown',
Expand Down
25 changes: 23 additions & 2 deletions test/dom/bindings.mocha.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(`
<view is="box"/>
`);
function Box() {}
Box.view = {
is: 'box',
source:`
<index:>
<div as="${badKey}">one</div>
`
};
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('<div>one</div>');
});
});
});

function testArray(itemTemplate, itemData) {
it('each on path', function() {
Expand Down

0 comments on commit 24524e9

Please sign in to comment.