Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cannot import @agoric/harden under SES-0.7.4 without noTameRegExp #237

Closed
warner opened this issue Mar 21, 2020 · 2 comments
Closed

cannot import @agoric/harden under SES-0.7.4 without noTameRegExp #237

warner opened this issue Mar 21, 2020 · 2 comments

Comments

@warner
Copy link
Contributor

warner commented Mar 21, 2020

The following CJS program:

const { lockdown } = require('ses');
lockdown( { noTameError: true });
require('@agoric/harden'); // fails

when run in an initially-empty directory where I've done yarn add esm ses@0.7.4 @agoric/harden@0.0.4, yields the following error:

$ node -r esm t.js
/tmp/t6/node_modules/@agoric/harden/dist/harden.cjs.js:1
TypeError: RegExp.prototype.source getter called on non-RegExp object
    at Object.get source [as source] (<anonymous>)
    at Object.toString (<anonymous>)
    at /tmp/t6/node_modules/@agoric/harden/dist/harden.cjs.js:280:47
    at Array.forEach (<anonymous>)
    at getAnonIntrinsics (/tmp/t6/node_modules/@agoric/harden/dist/harden.cjs.js:266:20)
    at buildTable (/tmp/t6/node_modules/@agoric/harden/dist/harden.cjs.js:1410:22)
    at Object.<anonymous> (/tmp/t6/node_modules/@agoric/harden/dist/harden.cjs.js:1430:22)
    at Object.<anonymous> (/tmp/t6/node_modules/@agoric/harden/dist/harden.cjs.js:1)

The line in question (harden.cjs.js line 280) is trying to toString an undeniable:

  const undeniableTuples = [
    ['Object.prototype', Object.prototype, {}],
    ['Function.prototype', Function.prototype, function foo() {}],
    ['Array.prototype', Array.prototype, []],
    ['RegExp.prototype', RegExp.prototype, /x/],
    ['Boolean.prototype', Boolean.prototype, true],
    ['Number.prototype', Number.prototype, 1],
    ['String.prototype', String.prototype, 'x'],
    ['%Generator%', Generator, aStrictGenerator],
    ['%AsyncGenerator%', AsyncGenerator, aStrictAsyncGenerator],
    ['%AsyncFunction%', AsyncFunctionPrototype, aStrictAsyncFunction],
  ];

  undeniableTuples.forEach(tuple => {
    const name = tuple[0];
    const undeniable = tuple[1];
    let start = tuple[2];
    if (start === undefined) {
      return;
    }
    start = Object(start);
    if (undeniable === start) {
      return;
    }
    if (undeniable === getProto(start)) {
      return;
    }
    throw new Error(`Unexpected undeniable: ${undeniable}`);
  });

If I change the ${undeniable} to ${name}, it reveals the unexpected undeniable:

Error: Unexpected undeniable: RegExp.prototype

I'm guessing that our fix for #230 was incomplete somehow, and the check that harden does is confused. There are two problems here: the first triggering this Error, and the second is that the undeniable in question throws a second error when the first one tries to call toString on RegExp.prototype.

@warner
Copy link
Contributor Author

warner commented Mar 21, 2020

Oh and of course the problem goes away if I add noTameRegExp: true to the lockdown() options, as you'd expect.

erights added a commit that referenced this issue Mar 23, 2020
Bad RegExp taming revealed by #237 

Exclude similar but new test failures
@erights
Copy link
Contributor

erights commented Mar 23, 2020

Fixed in #243

@erights erights closed this as completed Mar 23, 2020
warner added a commit to Agoric/agoric-sdk that referenced this issue Mar 24, 2020
refs #739

Note, for unit tests, `install-ses.js` disables some SES security features,
one because of a bug in SES (endojs/endo#237), and another because
`rollup` uses Math.random for reasons that are not clear. We'll need to fix
or find workarounds for both: eventually this test should work without those
two patches.
warner added a commit to Agoric/agoric-sdk that referenced this issue Mar 25, 2020
refs #739

Note, for unit tests, `install-ses.js` disables some SES security features,
one because of a bug in SES (endojs/endo#237), and another because
`rollup` uses Math.random for reasons that are not clear. We'll need to fix
or find workarounds for both: eventually this test should work without those
two patches.
warner added a commit to Agoric/agoric-sdk that referenced this issue Mar 26, 2020
refs #739

Note, for unit tests, `install-ses.js` disables some SES security features,
one because of a bug in SES (endojs/endo#237), and another because
`rollup` uses Math.random for reasons that are not clear. We'll need to fix
or find workarounds for both: eventually this test should work without those
two patches.
warner added a commit to Agoric/agoric-sdk that referenced this issue Mar 27, 2020
refs #739

Note, for unit tests, `install-ses.js` disables some SES security features,
one because of a bug in SES (endojs/endo#237), and another because
`rollup` uses Math.random for reasons that are not clear. We'll need to fix
or find workarounds for both: eventually this test should work without those
two patches.
warner added a commit to Agoric/agoric-sdk that referenced this issue Mar 27, 2020
refs #739

Note, for unit tests, `install-ses.js` disables some SES security features,
one because of a bug in SES (endojs/endo#237), and another because
`rollup` uses Math.random for reasons that are not clear. We'll need to fix
or find workarounds for both: eventually this test should work without those
two patches.
warner added a commit to Agoric/agoric-sdk that referenced this issue Mar 27, 2020
refs #739

Note, for unit tests, `install-ses.js` disables some SES security features,
one because of a bug in SES (endojs/endo#237), and another because
`rollup` uses Math.random for reasons that are not clear. We'll need to fix
or find workarounds for both: eventually this test should work without those
two patches.
warner added a commit to Agoric/agoric-sdk that referenced this issue Apr 1, 2020
refs #739

Note, for unit tests, `install-ses.js` disables some SES security features,
one because of a bug in SES (endojs/endo#237), and another because
`rollup` uses Math.random for reasons that are not clear. We'll need to fix
or find workarounds for both: eventually this test should work without those
two patches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants