Guard against prototype pollution

In several places, strings used as object keys can cause prototype pollution. While these strings are almost always developer-controlled, not end-user-controlled, it's still good to be defensive.

.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() {