fix(end-to-end): metering works for some malicious code

packages/transform-metering/package.json

@@ -16,7 +16,7 @@
     "esm": "^3.2.5",
     "rollup": "^1.16.6",
     "rollup-plugin-node-resolve": "^5.2.0",
-    "ses": "^0.6.3",
+    "ses": "^0.6.4",
     "tap-spec": "^5.0.0",
     "tape": "^4.9.2",
     "tape-promise": "^4.0.0"

packages/transform-metering/src/constants.js

@@ -14,4 +14,4 @@ export const DEFAULT_REGEXP_ID_PREFIX = '$h\u200d_re_';
 export const DEFAULT_COMBINED_METER = 1e6;
 export const DEFAULT_ALLOCATE_METER = true;
 export const DEFAULT_COMPUTE_METER = true;
-export const DEFAULT_STACK_METER = 32000;
+export const DEFAULT_STACK_METER = 8000;

packages/transform-metering/src/endow.js

@@ -1,10 +1,17 @@
 import harden from '@agoric/harden';
-import RE2 from 're2';
-
 import * as c from './constants';
 
-const { create, defineProperties, entries, fromEntries,
-  getOwnPropertyDescriptors, getPrototypeOf } = Object;
+// We'd like to import this, but RE2 is cjs
+const RE2 = require('re2');
+
+const {
+  create,
+  defineProperties,
+  entries,
+  fromEntries,
+  getOwnPropertyDescriptors,
+  getPrototypeOf,
+} = Object;
 
 export function makeMeteringEndowments(
   meter,
@@ -13,15 +20,15 @@ export function makeMeteringEndowments(
   overrideMeterId = c.DEFAULT_METER_ID,
 ) {
   const wrapped = new WeakMap();
+  wrapped.set(meter, meter);
   const meterId = overrideMeterId;
 
   const wrapDescriptor = desc =>
-    fromEntries(entries(desc).map(([k, v]) =>
-      [k, wrap(v)]
-    ));
+    fromEntries(entries(desc).map(([k, v]) => [k, wrap(v)]));
 
+  const shadowedRegexp = globalsToShadow.RegExp;
   function wrap(target) {
-    if (target === globalsToShadow.RegExp) {
+    if (shadowedRegexp !== undefined && target === shadowedRegexp) {
       // Replace the RegExp object with RE2.
       target = RE2;
     }
@@ -70,19 +77,21 @@ export function makeMeteringEndowments(
     wrapped.set(wrapper, wrapper);
 
     // Assign the wrapped descriptors to the wrapper.
-    const descs = fromEntries(entries(getOwnPropertyDescriptors(target))
-      .map(([k, v]) => [k, wrapDescriptor(v)]));
+    const descs = fromEntries(
+      entries(getOwnPropertyDescriptors(target)).map(([k, v]) => [
+        k,
+        wrapDescriptor(v),
+      ]),
+    );
     defineProperties(wrapper, descs);
     return wrapper;
   }
 
   // Shadow the wrapped globals with the wrapped endowments.
   const shadowDescs = create(null);
-  entries(getOwnPropertyDescriptors(globalsToShadow)).forEach(
-    ([p, desc]) => {
-      shadowDescs[p] = wrapDescriptor(desc);
-    },
-  );
+  entries(getOwnPropertyDescriptors(globalsToShadow)).forEach(([p, desc]) => {
+    shadowDescs[p] = wrapDescriptor(desc);
+  });
 
   entries(getOwnPropertyDescriptors(endowments)).forEach(([p, desc]) => {
     // We wrap the endowment descriptors, too.
@@ -100,5 +109,6 @@ export function makeMeteringEndowments(
   };
 
   // Package up these endowments as an object.
-  return create(null, shadowDescs);
+  const e = create(null, shadowDescs);
+  return e;
 }

packages/transform-metering/src/meter.js

@@ -11,14 +11,16 @@ const bigIntWord = typeof BigInt !== 'undefined' && BigInt(1 << 32);
 const bigIntZero = bigIntWord && BigInt(0);
 
 // Stop deducting when we reach a negative number.
-const makeCounter = balance => {
+const makeCounter = initBalance => {
+  let balance = initBalance;
   const counter = increment => {
     if (balance > 0) {
       balance += increment;
     }
     return balance;
   };
-  counter.reset = newBalance => (balance = newBalance);
+  counter.reset = (newBalance = undefined) =>
+    (balance = newBalance === undefined ? initBalance : newBalance);
   return counter;
 };
 
@@ -174,7 +176,7 @@ export function makeMeterAndResetters(maxima = {}) {
   // Allocate meters need both stack and compute meters.
   const meterAllocate = makeAllocateMeter(maybeAbort, meter, allocateCounter);
 
-  const makeResetter = cnt => newBalance => {
+  const makeResetter = cnt => (newBalance = undefined) => {
     maybeAbort.reset();
     if (cnt) {
       cnt.reset(newBalance);

packages/transform-metering/src/transform.js

@@ -70,8 +70,11 @@ export function makeMeteringTransformer(
     const visitor = {
       // Ensure meter identifiers are generated by us, or abort.
       Identifier(path) {
-        if ((path.node.name === meterId || path.node.name.startsWith(regexpIdPrefix))
-            && !path.node[METER_GENERATED]) {
+        if (
+          (path.node.name === meterId ||
+            path.node.name.startsWith(regexpIdPrefix)) &&
+          !path.node[METER_GENERATED]
+        ) {
           throw path.buildCodeFrameError(
             `Identifier ${path.node.name} is reserved for metering code`,
           );
@@ -134,7 +137,13 @@ const ${reid}=RegExp(${JSON.stringify(pattern)},${JSON.stringify(flags)});`);
 
   const meteringTransform = {
     rewrite(ss) {
-      const { source, endowments } = ss;
+      const { src: source, endowments } = ss;
+      if (!endowments[meterId]) {
+        return ss;
+      }
+
+      // Meter how much source code they want to use.
+      endowments[meterId][c.METER_COMPUTE](source.length);
 
       // Do the actual transform.
       const ast = parser(source);
@@ -158,14 +167,11 @@ const ${reid}=RegExp(${JSON.stringify(pattern)},${JSON.stringify(flags)});`);
           ? `(function(){${reSource}return ${maybeSource}})()`
           : `${reSource}${maybeSource}`;
 
-      // Meter how much source code they created.
-      endowments[meterId][c.METER_COMPUTE](actualSource.length);
-
+      // console.log('metered source:', actualSource);
       return {
         ...ss,
         ast,
-        endowments,
-        source: actualSource,
+        src: actualSource,
       };
     },
   };

packages/transform-metering/test/test-transform.js

@@ -17,16 +17,16 @@ test('meter transform', async t => {
     const rewrite = (source, testName) => {
       let cMeter;
       const ss = meteringTransform.rewrite({
-        source,
+        src: source,
         endowments: {
           [meterId]: {
-            [c.METER_COMPUTE]: units => cMeter = units,
+            [c.METER_COMPUTE]: units => (cMeter = units),
           },
         },
         sourceType: 'script',
       });
-      t.equals(cMeter, ss.source.length, `compute meter updated ${testName}`);
-      return ss.source;
+      t.equals(cMeter, source.length, `compute meter updated ${testName}`);
+      return ss.src;
     };
 
     t.throws(

packages/transform-metering/test/test-zzz-e2e.js

@@ -0,0 +1,66 @@
+/* global globalThis */
+/* eslint-disable no-await-in-loop */
+import test from 'tape-promise/tape';
+import * as babelCore from '@babel/core';
+import SES from 'ses';
+
+import {
+  makeMeterAndResetters,
+  makeMeteringEndowments,
+  makeMeteringTransformer,
+} from '../src/index';
+
+test('metering end-to-end', async t => {
+  try {
+    const [meter, reset] = makeMeterAndResetters();
+    const { meterId, meteringTransform } = makeMeteringTransformer(babelCore);
+    const endowments = makeMeteringEndowments(meter, globalThis, {}, meterId);
+    const transforms = [meteringTransform];
+
+    const s = SES.makeSESRootRealm({ transforms });
+
+    const myEval = src => {
+      Object.values(reset).forEach(r => r());
+      return s.evaluate(src, endowments);
+    };
+
+    const src1 = `123; 456;`;
+    t.equals(myEval(src1), 456, 'trivial source succeeds');
+
+    const src2 = `\
+function f() {
+  f();
+  return 1;
+}
+f();
+`;
+    t.throws(
+      () => myEval(src2),
+      /Stack meter exceeded/,
+      'stack overflow fails',
+    );
+
+    const src3 = `\
+while (true) {}
+`;
+    t.throws(
+      () => myEval(src3),
+      /Compute meter exceeded/,
+      'infinite loop fails',
+    );
+
+    const src4 = `\
+/(x+x+)+y/.test('x'.repeat(10000));
+`;
+    t.equals(myEval(src4), false, `catastrophic backtracking doesn't happen`);
+
+    const src5 = `\
+new Array(1e6).map(Object.create)
+`;
+    t.throws(() => myEval(src5), RangeError, 'long map fails');
+  } catch (e) {
+    t.isNot(e, e, 'unexpected exception');
+  } finally {
+    t.end();
+  }
+});