Merge pull request #630 from derbyjs/proto-pollution-guard

Guard against prototype pollution

.eslintrc

@@ -10,6 +10,7 @@
  "document": false
  },
  "root": true,
+ "ignorePatterns": ["dist/"],
  "rules": {
  "comma-style": [
  2,

src/App.ts

@@ -18,6 +18,7 @@ import { Page, type PageBase } from './Page';
 import { PageParams, routes } from './routes';
 import * as derbyTemplates from './templates';
 import { type Views } from './templates/templates';
+import { checkKeyIsSafe } from './templates/util';
 
 const { templates } = derbyTemplates;
 
@@ -122,6 +123,7 @@ export abstract class AppBase<T = object> extends EventEmitter {
 
  // TODO: DRY. This is copy-pasted from ./templates
  const mapName = viewName.replace(/:index$/, '');
+ checkKeyIsSafe(mapName);
  const currentView = this.views.nameMap[mapName];
  const currentConstructor = (currentView && currentView.componentFactory) ?
  currentView.componentFactory.constructorFn :

src/PageForServer.ts

@@ -84,11 +84,11 @@ function stringifyBundle(bundle) {
 
 // TODO: Cleanup; copied from tracks
 function pageParams(req) {
- const params = {
+ const params = Object.create(null, {
  url: req.url,
  body: req.body,
  query: req.query,
- };
+ });
  for (const key in req.params) {
  params[key] = req.params[key];
  }

src/components.ts

@@ -16,6 +16,7 @@ import derbyTemplates = require('./templates');
 import { Context } from './templates/contexts';
 import { Expression } from './templates/expressions';
 import { Attribute, Binding } from './templates/templates';
+import { checkKeyIsSafe } from './templates/util';
 const { expressions, templates } = derbyTemplates;
 const slice = [].slice;
 
@@ -332,10 +333,12 @@ export abstract class Component<T = object> extends Controller<T> {
  }
 
  setAttribute(key: string, value: Attribute) {
+ checkKeyIsSafe(key);
  this.context.parent.attributes[key] = value;
  }
 
  setNullAttribute(key: string, value: Attribute) {
+ checkKeyIsSafe(key);
  const attributes = this.context.parent.attributes;
  if (attributes[key] == null) attributes[key] = value;
  }
@@ -354,6 +357,7 @@ export class ComponentAttribute<T = object> {
  key: string;
 
  constructor(expression: Expression, model: ChildModel<T>, key: string) {
+ checkKeyIsSafe(key);
  this.expression = expression;
  this.model = model;
  this.key = key;

src/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());
 };
 

src/parsing/createPathExpression.ts

@@ -2,6 +2,7 @@ import * as esprima from 'esprima-derby';
 import type * as estree from 'estree';
 
 import { expressions, operatorFns } from '../templates';
+import { checkKeyIsSafe } from '../templates/util';
 const { Syntax } = esprima;
 
 export function createPathExpression(source: string) {
@@ -221,6 +222,7 @@ function reduceObjectExpression(node: estree.ObjectExpression) {
  if (isProperty(property)) {
  const key = getKeyName(property.key);
  const expression = reduce(property.value);
+ checkKeyIsSafe(key);
  properties[key] = expression;
  if (isLiteral && expression instanceof expressions.LiteralExpression) {
  literal[key] = expression.value;

src/parsing/index.ts

@@ -8,6 +8,7 @@ import { App, AppBase } from '../App';
 import { templates, expressions } from '../templates';
 import { Expression } from '../templates/expressions';
 import { MarkupHook, View } from '../templates/templates';
+import { checkKeyIsSafe } from '../templates/util';
 
 export { createPathExpression } from './createPathExpression';
 export { markup } from './markup';
@@ -122,6 +123,7 @@ function parseHtmlStart(tag: string, tagName: string, attributes: Record<string,
 function parseAttributes(attributes: Record<string, string>) {
  let attributesMap;
  for (const key in attributes) {
+ checkKeyIsSafe(key);
  if (!attributesMap) attributesMap = {};
 
  const value = attributes[key];
@@ -384,6 +386,7 @@ function attributeValueFromContent(content, isWithin) {
 function viewAttributesFromElement(element) {
  const viewAttributes = {};
  for (const key in element.attributes) {
+ checkKeyIsSafe(key);
  const attribute = element.attributes[key];
  const camelCased = dashToCamelCase(key);
  viewAttributes[camelCased] =
@@ -555,6 +558,7 @@ function parseNameAttribute(element) {
 
 function parseAttributeElement(element, name, viewAttributes) {
  const camelName = dashToCamelCase(name);
+ checkKeyIsSafe(camelName);
  const isWithin = element.attributes && element.attributes.within;
  viewAttributes[camelName] = attributeValueFromContent(element.content, isWithin);
 }
@@ -564,6 +568,7 @@ function createAttributesExpression(attributes) {
  const literalAttributes = {};
  let isLiteral = true;
  for (const key in attributes) {
+ checkKeyIsSafe(key);
  const attribute = attributes[key];
  if (attribute instanceof expressions.Expression) {
  dynamicAttributes[key] = attribute;
@@ -588,6 +593,7 @@ function parseArrayElement(element, name, viewAttributes) {
  delete attributes.within;
  const expression = createAttributesExpression(attributes);
  const camelName = dashToCamelCase(name);
+ checkKeyIsSafe(camelName);
  const viewAttribute = viewAttributes[camelName];
 
  // If viewAttribute is already an ArrayExpression, push the expression for
@@ -662,6 +668,7 @@ function viewAttributesFromExpression(expression) {
 
  const viewAttributes = {};
  for (const key in object) {
+ checkKeyIsSafe(key);
  const value = object[key];
  viewAttributes[key] =
  (value instanceof expressions.LiteralExpression) ? value.value :

src/templates/expressions.ts

@@ -4,7 +4,7 @@ import { type Context } from './contexts';
 import { DependencyOptions } from './dependencyOptions';
 import * as operatorFns from './operatorFns';
 import { ContextClosure, Dependency, Template } from './templates';
-import { concat } from './util';
+import { checkKeyIsSafe, concat } from './util';
 import { Component } from '../components';
 
 type SegmentOrContext = string | number | { item: number } | Context;
@@ -88,6 +88,7 @@ function renderArrayProperties(array: Renderable[], context: Context) {
 function renderObjectProperties(object: Record<string, Renderable>, context: Context): Record<string, any> {
  const out = {};
  for (const key in object) {
+ checkKeyIsSafe(key);
  out[key] = renderValue(object[key], context);
  }
  return out;
@@ -547,6 +548,7 @@ export class ObjectExpression extends Expression {
  get(context: Context) {
  const object = {};
  for (const key in this.properties) {
+ checkKeyIsSafe(key);
  const value = this.properties[key].get(context);
  object[key] = value;
  }

src/templates/templates.ts

@@ -6,7 +6,7 @@ if (typeof require === 'function') {
 import { type Context } from './contexts';
 import { DependencyOptions } from './dependencyOptions';
 import { type Expression } from './expressions';
-import { concat, hasKeys, traverseAndCreate } from './util';
+import { checkKeyIsSafe, concat, hasKeys, traverseAndCreate } from './util';
 import { Component } from '../components';
 import { Controller } from '../Controller';
 
@@ -1369,6 +1369,7 @@ function emitRemoved(context: Context, node: Node, ignore: Binding) {
 }
 
 function emitRemovedBinding(context: Context, ignore: Binding, node: Node, property: string) {
+ checkKeyIsSafe(property);
  const binding = node[property];
  if (binding) {
  node[property] = null;
@@ -1444,6 +1445,7 @@ export class AttributeBinding extends Binding {
  name: string;
 
  constructor(template: DynamicAttribute, context: Context, element: globalThis.Element, name: string) {
+ checkKeyIsSafe(name);
  super();
  this.template = template;
  this.context = context;
@@ -1728,6 +1730,7 @@ export class Marker extends Comment {
 function ViewAttributesMap(source: string) {
  const items = source.split(/\s+/);
  for (let i = 0, len = items.length; i < len; i++) {
+ checkKeyIsSafe(items[i]);
  this[items[i]] = true;
  }
 }
@@ -1736,6 +1739,7 @@ function ViewArraysMap(source: string) {
  const items = source.split(/\s+/);
  for (let i = 0, len = items.length; i < len; i++) {
  const item = items[i].split('/');
+ checkKeyIsSafe(item[0]);
  this[item[0]] = item[1] || item[0];
  }
 }
@@ -2159,6 +2163,7 @@ export class Views {
 
  register(name: string, source: string, options?: ViewOptions) {
  const mapName = name.replace(/:index$/, '');
+ checkKeyIsSafe(mapName);
  let view = this.nameMap[mapName];
  if (view) {
  // Recreate the view if it already exists. We re-apply the constructor
@@ -2173,6 +2178,7 @@ export class Views {
  this.nameMap[mapName] = view;
  // TODO: element is deprecated and should be removed with Derby 0.6.0
  const tagName = options && (options.tag || options.element);
+ checkKeyIsSafe(tagName);
  if (tagName) this.tagMap[tagName] = view;
  return view;
  }
@@ -2323,6 +2329,7 @@ abstract class AsPropertyBase<T> extends MarkupHook<T> {
 
  emit(context: Context, target: T) {
  const node = traverseAndCreate(context.controller, this.segments);
+ checkKeyIsSafe(this.lastSegment);
  node[this.lastSegment] = target;
  this.addListeners(target, node, this.lastSegment);
  }
@@ -2376,8 +2383,10 @@ export class AsObject extends AsProperty {
 
  emit(context: Context, target: any) {
  const node = traverseAndCreate(context.controller, this.segments);
+ checkKeyIsSafe(this.lastSegment);
  const object = node[this.lastSegment] || (node[this.lastSegment] = {});
  const key = this.keyExpression.get(context);
+ checkKeyIsSafe(key);
  object[key] = target;
  this.addListeners(target, object, key);
  }
@@ -2398,6 +2407,7 @@ abstract class AsArrayBase<T> extends AsPropertyBase<T> {
 
  emit(context: Context, target: any) {
  const node = traverseAndCreate(context.controller, this.segments);
+ checkKeyIsSafe(this.lastSegment);
  const array = node[this.lastSegment] || (node[this.lastSegment] = []);
 
  // Iterate backwards, since rendering will usually append

src/templates/util.ts

@@ -1,3 +1,16 @@
+const objectProtoPropNames = Object.create(null);
+Object.getOwnPropertyNames(Object.prototype).forEach(function(prop) {
+ if (prop !== '__proto__') {
+ objectProtoPropNames[prop] = true;
+ }
+});
+
+export function checkKeyIsSafe(key) {
+ if (key === '__proto__' || objectProtoPropNames[key]) {
+ throw new Error(`Unsafe key "${key}"`);
+ }
+}
+
 export function concat(a, b) {
  if (!a) return b;
  if (!b) return a;
@@ -17,6 +30,7 @@ export function traverseAndCreate(node, segments) {
  if (!len) return node;
  for (let i = 0; i < len; i++) {
  const segment = segments[i];
+ checkKeyIsSafe(segment);
  node = node[segment] || (node[segment] = {});
  }
  return node;

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(`
+ <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() {