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

passStytleOf helpers too lax with prototypes #4333

Closed
mhofman opened this issue Jan 20, 2022 · 0 comments · Fixed by endojs/endo#1250
Closed

passStytleOf helpers too lax with prototypes #4333

mhofman opened this issue Jan 20, 2022 · 0 comments · Fixed by endojs/endo#1250
Assignees
Labels
audit-zestival Vulnerability assessment of ERTP + Zoe bug Something isn't working marshal package: marshal security SwingSet package: SwingSet

Comments

@mhofman
Copy link
Member

mhofman commented Jan 20, 2022

Describe the bug

The top level of passStyleOf's algorithm ensures that objects it traverses are frozen. The prototype checks are left to the various pass style helpers. Some helpers simply verify that the prototype is a well know (and thus hardened) prototype. Others accept more complex objects, and some of their prototype checks do not enforce a hardened chain.

  • Remotable: Only checks that the prototype has the right shape, but doesn't require it to be frozen, or for the properties to not be accessors. This allows the prototype object to mutate itself (including by changing its proto chain) after it has been checked.
    See

    const checkRemotableProtoOf = (original, check = x => x) => {

  • Promise: Only checks that it's a promise, and not platform promise, aka not a Frankenstein promise with a different proto or own then.

Impact

The severity is mitigated by the logic of passStyleOf which once an object has been classified, doesn't recompute the pass style anymore. However there are a few places where this might have strange implications:

  • getInterfaceOf may return inconsistent values. For example it could return different values over time. I do not believe it's value is used anywhere currently.
  • convertValToSlot (e.g. in liveSlots) does some value type checks as well, and may be tricked into have a remotable being processed as a promise.

Expected behavior

Prototypes checks being stricter.
No confusion possible between remotable types.

Platform Environment

  • what version of the Agoric-SDK are you using? 72cc8d6bcf428596653593708959446fb0a29596
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit-zestival Vulnerability assessment of ERTP + Zoe bug Something isn't working marshal package: marshal security SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants