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

track-turns doesn't guard against overridden .catch method #1244

Open
warner opened this issue Aug 4, 2022 · 2 comments
Open

track-turns doesn't guard against overridden .catch method #1244

warner opened this issue Aug 4, 2022 · 2 comments
Assignees
Labels
confinement Pertaining to confinement of guest programs. endo kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024

Comments

@warner
Copy link
Contributor

warner commented Aug 4, 2022

@erights and I were investigating a potential object-retention problem with the track-turns implementation, when we noticed that the following line:

Promise.resolve(result).catch(reason => {

would be vulnerable to the wrapped function's return value being a real Promise but whose .catch method had been overridden. I haven't tried to build an exploit, but I think it would look something like this:

function foo(args) {
   stuff(args);
   const fakePromise = new Promise((res, rej) => stuff(res, resj));
   const oldCatch = fakePromise.catch;
    fakePromise.catch = handler => {
      console.log(`handler I'm not supposed to be able to see:`, handler);
      return oldCatch(handler);
    };
    return fakePromise;
}
const target = Far('', { foo });

E(target).foo(args);

The track-turns mechanism is supposed to be invisible to user code, but the fake catch would get to see the track-turns internal function that adds a rejection note to any error it emits.

I'm guessing the way to fix this would be to replace the Promise.resolve(result).catch(..) with an E.when(result).catch(..), but I'm not positive.

cc @erights @michaelfig

@erights
Copy link
Contributor

erights commented Aug 4, 2022

cc @mhofman

@erights
Copy link
Contributor

erights commented Aug 4, 2022

I'm guessing the way to fix this would be to replace the Promise.resolve(result).catch(..) with an E.when(result).catch(..), but I'm not positive.

except that we need to avoid the infinite regress of E.when using trackTurns in its implementation ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confinement Pertaining to confinement of guest programs. endo kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024
Projects
None yet
Development

No branches or pull requests

4 participants