fix(amplify-velocity-template): consistent handling of reserved property names in reference resolution

packages/amplify-velocity-template/src/compile/references.js

@@ -1,28 +1,11 @@
+var reserved = require('./reserved-properties');
+var isBlockedProperty = reserved.isBlockedProperty;
+var safeGetProperty = reserved.safeGetProperty;
+var safeSetProperty = reserved.safeSetProperty;
+
 module.exports = function (Velocity, utils) {
   'use strict';
 
-  var BLOCKED_PROPERTIES = [
-    'constructor',
-    '__proto__',
-    'prototype',
-    '__defineGetter__',
-    '__defineSetter__',
-    '__lookupGetter__',
-    '__lookupSetter__',
-  ];
-
-  /**
-   * Determine whether a property name traverses the JavaScript prototype
-   * chain (e.g. constructor/__proto__/prototype) and must be blocked so it
-   * cannot be used to reach the Function constructor and escape the template
-   * sandbox.
-   * @param {string} name property name being resolved
-   * @returns {boolean} true when the name is disallowed
-   */
-  function isBlockedProperty(name) {
-    return typeof name === 'string' && BLOCKED_PROPERTIES.indexOf(name) !== -1;
-  }
-
   function getSize(obj) {
     if (utils.isArray(obj)) {
       return obj.length;
@@ -74,6 +57,10 @@ module.exports = function (Velocity, utils) {
       return base[property];
     }
 
+    if (isBlockedProperty(property)) {
+      return undefined;
+    }
+
     var letter = property.charCodeAt(0);
     var isUpper = letter < 91;
     var ret = base[property];
@@ -92,6 +79,12 @@ module.exports = function (Velocity, utils) {
       property = String.fromCharCode(letter).toUpperCase() + property.slice(1);
     }
 
+    // Re-check after case normalization so getConstructor() cannot map to
+    // "Constructor" then lowercase to "constructor" and reach the prototype chain.
+    if (isBlockedProperty(property)) {
+      return undefined;
+    }
+
     return base[property];
   }
 
@@ -128,12 +121,12 @@ module.exports = function (Velocity, utils) {
       var isSilent = this.silence || ast.leader === '$!';
       var isfn = ast.args !== undefined;
       var context = this.context;
-      var ret = context[ast.id];
+      var ret = safeGetProperty(context, ast.id);
       var local = this.getLocal(ast);
 
       var text = Velocity.Helper.getRefText(ast);
 
-      if (text in context) {
+      if (!isBlockedProperty(text) && Object.prototype.hasOwnProperty.call(context, text)) {
         return ast.prue && escape ? convert(context[text]) : context[text];
       }
 
@@ -175,6 +168,12 @@ module.exports = function (Velocity, utils) {
       var local = this.local;
       var ret = false;
 
+      // Local scopes are plain objects, so `id in _local` would also match
+      // inherited names like constructor and resolve them to Object.
+      if (isBlockedProperty(id)) {
+        return { value: ret, isLocaled: false };
+      }
+
       var isLocaled = utils.some(
         this.conditions,
         function (contextId) {
@@ -216,7 +215,7 @@ module.exports = function (Velocity, utils) {
       if (type === 'method') {
         ret = this.getPropMethod(property, baseRef, ast);
       } else if (type === 'property') {
-        ret = isBlockedProperty(id) ? undefined : baseRef[id];
+        ret = safeGetProperty(baseRef, id);
       } else {
         ret = this.getPropIndex(property, baseRef);
       }
@@ -238,7 +237,7 @@ module.exports = function (Velocity, utils) {
         key = ast.value;
       }
 
-      return isBlockedProperty(key) ? undefined : baseRef[key];
+      return safeGetProperty(baseRef, key);
     },
 
     /**
@@ -258,7 +257,7 @@ module.exports = function (Velocity, utils) {
       }
 
       if (id === 'set' && !(id in baseRef)) {
-        baseRef[this.getLiteral(property.args[0])] = this.getLiteral(property.args[1]);
+        safeSetProperty(baseRef, this.getLiteral(property.args[0]), this.getLiteral(property.args[1]));
         return '';
       }
 
@@ -269,7 +268,9 @@ module.exports = function (Velocity, utils) {
 
       // setter 处理
       if (id.indexOf('set') === 0 && !baseRef[id]) {
-        baseRef[id.slice(3)] = this.getLiteral(property.args[0]);
+        if (!safeSetProperty(baseRef, id.slice(3), this.getLiteral(property.args[0]))) {
+          return '';
+        }
         // $page.setName(123)
         baseRef.toString = function () {
           return '';
@@ -289,7 +290,9 @@ module.exports = function (Velocity, utils) {
       } else if (id === 'size' && !baseRef[id]) {
         return getSize(baseRef);
       } else if (id === 'put' && !baseRef[id]) {
-        return (baseRef[this.getLiteral(property.args[0])] = this.getLiteral(property.args[1]));
+        var putKey = this.getLiteral(property.args[0]);
+        var putValue = this.getLiteral(property.args[1]);
+        return safeSetProperty(baseRef, putKey, putValue) ? putValue : undefined;
       } else if (id === 'add' && !baseRef[id] && typeof baseRef.push === 'function') {
         return baseRef.push(this.getLiteral(property.args[0]));
       } else if (id === 'remove') {
@@ -306,8 +309,12 @@ module.exports = function (Velocity, utils) {
           baseRef.splice(indexToRemove, 1);
           return ret;
         } else if (utils.isObject(baseRef)) {
-          ret = baseRef[this.getLiteral(property.args[0])];
-          delete baseRef[this.getLiteral(property.args[0])];
+          var removeKey = this.getLiteral(property.args[0]);
+          if (isBlockedProperty(removeKey)) {
+            return undefined;
+          }
+          ret = baseRef[removeKey];
+          delete baseRef[removeKey];
           return ret;
         }
 

packages/amplify-velocity-template/src/compile/reserved-properties.js

@@ -0,0 +1,60 @@
+'use strict';
+
+// Prototype-chain names that templates must not read or write, shared by
+// reference resolution (references.js) and #set (set.js). Keeping one list
+// stops the copies drifting apart when a name is added to only one of them.
+var BLOCKED_PROPERTIES = [
+  'constructor',
+  '__proto__',
+  'prototype',
+  '__defineGetter__',
+  '__defineSetter__',
+  '__lookupGetter__',
+  '__lookupSetter__',
+];
+
+/**
+ * Determine whether a property name traverses the JavaScript prototype
+ * chain (e.g. constructor/__proto__/prototype) and must be blocked so it
+ * cannot be used to reach the Function constructor and escape the template
+ * sandbox.
+ * @param {string} name property name being resolved
+ * @returns {boolean} true when the name is disallowed
+ */
+function isBlockedProperty(name) {
+  return typeof name === 'string' && BLOCKED_PROPERTIES.indexOf(name) !== -1;
+}
+
+/**
+ * Guarded read: route template-controlled property reads through this so a
+ * new read path is protected by construction instead of by remembering to
+ * call isBlockedProperty.
+ * @param {object} base object being read
+ * @param {string|number} key property name or index from the template
+ * @returns {*} base[key], or undefined when the key is blocked
+ */
+function safeGetProperty(base, key) {
+  return isBlockedProperty(key) ? undefined : base[key];
+}
+
+/**
+ * Guarded write: route template-controlled property writes through this.
+ * @param {object} base object being written
+ * @param {string|number} key property name from the template
+ * @param {*} value value to assign
+ * @returns {boolean} true if written, false if the key was blocked
+ */
+function safeSetProperty(base, key, value) {
+  if (isBlockedProperty(key)) {
+    return false;
+  }
+  base[key] = value;
+  return true;
+}
+
+module.exports = {
+  BLOCKED_PROPERTIES: BLOCKED_PROPERTIES,
+  isBlockedProperty: isBlockedProperty,
+  safeGetProperty: safeGetProperty,
+  safeSetProperty: safeSetProperty,
+};

packages/amplify-velocity-template/src/compile/set.js

@@ -1,3 +1,8 @@
+var reserved = require('./reserved-properties');
+var isBlockedProperty = reserved.isBlockedProperty;
+var safeGetProperty = reserved.safeGetProperty;
+var safeSetProperty = reserved.safeSetProperty;
+
 module.exports = function (Velocity, utils) {
   /**
    * 变量设置
@@ -39,6 +44,10 @@ module.exports = function (Velocity, utils) {
         val = this.config.valueMapper(this.getLiteral(ast.equal[1]), valAst.type);
       }
 
+      if (isBlockedProperty(ref.id)) {
+        return;
+      }
+
       if (!ref.path) {
         context[ref.id] = val;
       } else {
@@ -63,10 +72,11 @@ module.exports = function (Velocity, utils) {
           }
 
           if (isEnd) {
-            return (baseRef[key] = val);
+            safeSetProperty(baseRef, key, val);
+            return true;
           }
 
-          baseRef = baseRef[key];
+          baseRef = safeGetProperty(baseRef, key);
 
           // such as
           // #set($a.d.c2 = 2)

packages/amplify-velocity-template/tests/reserved-property-names.test.js

@@ -0,0 +1,127 @@
+var Velocity = require('../src/velocity');
+require('should');
+
+describe('reserved property names in reference resolution', function () {
+  var render = Velocity.render;
+
+  afterEach(function () {
+    delete global.__vtlReserved;
+    delete Object.prototype.polluted;
+    delete Object.prototype.y;
+    delete Object.prototype.p;
+  });
+
+  it('does not resolve constructor via getConstructor() accessor', function () {
+    render('$!ctx.getConstructor()', { ctx: { name: 'foo' } }).should.eql('');
+  });
+
+  it('does not invoke a resolved constructor via getConstructor() chained call', function () {
+    global.__vtlReserved = false;
+    var vm = '$!ctx.getConstructor().getConstructor("globalThis.__vtlReserved = true").call()';
+    render(vm, { ctx: { name: 'foo' } }).should.eql('');
+    global.__vtlReserved.should.eql(false);
+  });
+
+  it('does not resolve constructor via isConstructor() accessor', function () {
+    render('$!ctx.isConstructor()', { ctx: { name: 'foo' } }).should.eql('');
+  });
+
+  it('does not resolve constructor via get("constructor")', function () {
+    render('$!ctx.get("constructor")', { ctx: { name: 'foo' } }).should.eql('');
+  });
+
+  it('does not resolve __proto__ via get("__proto__")', function () {
+    render('$!ctx.get("__proto__")', { ctx: { name: 'foo' } }).should.eql('');
+  });
+
+  it('does not write via #set on __proto__', function () {
+    render('#set($ctx.__proto__.polluted = "yes")', { ctx: { name: 'foo' } });
+    (({}).polluted === undefined).should.eql(true);
+  });
+
+  it('does not write via #set on constructor.prototype', function () {
+    render('#set($ctx.constructor.prototype.y = "yes")', { ctx: { name: 'foo' } });
+    (({}).y === undefined).should.eql(true);
+  });
+
+  it('does not write a reserved key via put("constructor", ...)', function () {
+    // Discriminating: without the guard, put() assigns an own "constructor"
+    // key (hasOwnProperty becomes true). put("__proto__", <string>) would be a
+    // JS no-op guarded or not, so "constructor" is used to actually exercise it.
+    var ctx = { name: 'foo' };
+    render('$!ctx.put("constructor", "yes")', { ctx: ctx });
+    ctx.hasOwnProperty('constructor').should.eql(false);
+  });
+
+  it('does not write a reserved key via set("constructor", ...)', function () {
+    var ctx = { name: 'foo' };
+    render('$!ctx.set("constructor", "yes")', { ctx: ctx });
+    ctx.hasOwnProperty('constructor').should.eql(false);
+  });
+
+  it('does not write a reserved key via setconstructor(...) accessor', function () {
+    // Discriminating: the setter path takes id.slice(3) verbatim with no
+    // re-casing, so the lowercase "setconstructor" produces the reserved key
+    // "constructor" that the guard actually matches. (Capitalized
+    // "setConstructor" slices to "Constructor", which is not reserved and never
+    // exercises the guard.) Without the guard this writes an own "constructor".
+    var ctx = { name: 'foo' };
+    render('$!ctx.setconstructor("yes")', { ctx: ctx });
+    ctx.hasOwnProperty('constructor').should.eql(false);
+  });
+
+  it('renders a rejected setconstructor(...) like other rejected writes', function () {
+    render('$ctx.setconstructor("yes")', { ctx: { name: 'foo' } }).should.eql('');
+  });
+
+  it('does not write through a reserved root identifier via #set', function () {
+    render('#set($__proto__.p = "x")', { ctx: { name: 'foo' } });
+    (({}).p === undefined).should.eql(true);
+  });
+
+  it('does not assign a reserved root identifier via #set', function () {
+    var context = { name: 'foo' };
+    render('#set($__proto__ = {"p": "x"})', context);
+    (Object.getPrototypeOf(context) === Object.prototype).should.eql(true);
+  });
+
+  it('does not resolve a reserved root identifier in a reference', function () {
+    render('$!constructor.assign($__proto__, {"p":"x"})', { ctx: { name: 'foo' } });
+    (({}).p === undefined).should.eql(true);
+  });
+
+  it('does not resolve a reserved root identifier from a #foreach scope', function () {
+    render('#foreach($i in [1])$!constructor.assign($__proto__, {"p":"x"})#end', {});
+    (({}).p === undefined).should.eql(true);
+  });
+
+  it('does not read or delete a reserved key via remove()', function () {
+    var vm = '$!ctx.remove("constructor").assign($ctx.remove("__proto__"), {"p":"x"})';
+    render(vm, { ctx: { name: 'foo' } });
+    (({}).p === undefined).should.eql(true);
+  });
+
+  it('still removes a normal key via remove()', function () {
+    var ctx = { name: 'foo', k: 'v' };
+    render('$ctx.remove("k")', { ctx: ctx }).should.eql('v');
+    ctx.hasOwnProperty('k').should.eql(false);
+  });
+
+  it('still resolves a normal root identifier, including inside #foreach', function () {
+    render('$a#foreach($i in [1])$a$i#end', { a: 'x' }).should.eql('xx1');
+  });
+
+  it('still resolves a normal getter accessor', function () {
+    render('$obj.getName()', { obj: { name: 'foo' } }).should.eql('foo');
+  });
+
+  it('still writes a normal property via #set', function () {
+    render('#set($ctx.bar = "baz")$ctx.bar', { ctx: { name: 'foo' } }).should.eql('baz');
+  });
+
+  it('still writes a normal property via put()', function () {
+    var map = {};
+    render('$!map.put("k", "v")', { map: map });
+    map.k.should.eql('v');
+  });
+});