Harden JS file address property access

lib/configuration/variables/sources/file.js

@@ -4,12 +4,10 @@ const ensureString = require('type/string/ensure');
 const isPlainFunction = require('type/plain-function/is');
 const path = require('path');
 const fsp = require('fs').promises;
-const { isProxy } = require('util').types;
 const yaml = require('js-yaml');
 const cloudformationSchema = require('../../../utils/serverless-utils/cloudformation-schema');
 const ServerlessError = require('../../../serverless-error');
-const { hasOwn } = require('../../../utils/safe-object');
-const { isUnsafePropertyKey } = require('../../../utils/object-path');
+const { canFollowProperty } = require('../../../utils/safe-object');
 
 const readFile = async (filePath, serviceDir) => {
   try {
@@ -153,13 +151,17 @@ module.exports = {
     for (const propertyKey of propertyKeys) {
       if (typeof result === 'string') result = await resolveVariablesInString(result);
       if (result == null) return { value: null };
-      if (!hasOwn(result, propertyKey)) {
-        // `hasOwnProperty` does not consult Proxy "has"/"get" traps, so virtual
-        // properties of Proxy-backed JS file results must be resolved through
-        // property access. Prototype internals stay blocked unless claimed as own.
-        if (!isProxy(result) || isUnsafePropertyKey(propertyKey)) return { value: null };
+      try {
+        if (!canFollowProperty(result, propertyKey)) return { value: null };
+        result = result[propertyKey];
+      } catch (error) {
+        throw new ServerlessError(
+          `Cannot resolve "${address}" out of "${path.basename(
+            filePath
+          )}": Property access errored with: ${error && error.stack ? error.stack : error}`,
+          'JS_FILE_PROPERTY_ACCESS_ERROR'
+        );
       }
-      result = result[propertyKey];
       if (result == null) return { value: null };
       if (!isResolvedByFunction) {
         if (isPlainFunction(result)) {

lib/utils/safe-object.js

@@ -1,11 +1,18 @@
 'use strict';
 
+const { isProxy } = require('util').types;
 const { isUnsafePropertyKey } = require('./object-path');
 
 const hasOwnProperty = Object.prototype.hasOwnProperty;
 
 const hasOwn = (object, key) => object != null && hasOwnProperty.call(object, key);
 
+// `hasOwnProperty` does not consult Proxy "has"/"get" traps, so virtual properties
+// of Proxy-backed values can only be reached via property access. Such access must
+// never traverse prototype internals, unless these are claimed as own properties.
+const canFollowProperty = (value, key) =>
+  hasOwn(value, key) || (isProxy(value) && !isUnsafePropertyKey(key));
+
 const dataDescriptor = (value) =>
   Object.assign(Object.create(null), {
     value,
@@ -54,6 +61,7 @@ const getOwnByPath = (source, path) => {
 };
 
 module.exports = {
+  canFollowProperty,
   createRegistry,
   getOwnByPath,
   hasOwn,

test/unit/lib/configuration/variables/sources/file.test.js

@@ -54,6 +54,8 @@ describe('test/unit/lib/configuration/variables/sources/file.test.js', () => {
         '${file(file-property-function-errored-non-error.js):property}',
       jsFilePropertyFunctionAccessUnresolvableProperty:
         '${file(file-property-function-access-unresolvable-property.js):property}',
+      jsFilePropertyProxyTrapErrored: '${file(file-property-access-errored.js):proxy.boom}',
+      jsFilePropertyGetterErrored: '${file(file-property-access-errored.js):getter.boom}',
       jsFilePropertyPromise: '${file(file-property-promise.js):property}',
       notFile: '${file(dir.yaml)}',
       noParams: '${file:}',
@@ -207,6 +209,18 @@ describe('test/unit/lib/configuration/variables/sources/file.test.js', () => {
       'VARIABLE_RESOLUTION_ERROR'
     ));
 
+  it('should report with an error a Proxy "get" trap that crashes during address resolution', () => {
+    const { error } = variablesMeta.get('jsFilePropertyProxyTrapErrored');
+    expect(error.code).to.equal('VARIABLE_RESOLUTION_ERROR');
+    expect(error.message).to.include('Property access errored with');
+  });
+
+  it('should report with an error an own getter that crashes during address resolution', () => {
+    const { error } = variablesMeta.get('jsFilePropertyGetterErrored');
+    expect(error.code).to.equal('VARIABLE_RESOLUTION_ERROR');
+    expect(error.message).to.include('Property access errored with');
+  });
+
   it('should report with an error non file paths', () =>
     expect(variablesMeta.get('notFile').error.code).to.equal('VARIABLE_RESOLUTION_ERROR'));
 

test/unit/lib/configuration/variables/sources/fixture/file-property-access-errored.js

@@ -0,0 +1,19 @@
+'use strict';
+
+module.exports = {
+  proxy: new Proxy(
+    {},
+    {
+      get(target, key) {
+        if (key === 'boom') throw new Error('Proxy get trap crashed');
+        return target[key];
+      },
+    }
+  ),
+  getter: Object.defineProperty({}, 'boom', {
+    enumerable: true,
+    get() {
+      throw new Error('Getter crashed');
+    },
+  }),
+};