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

Need safer expansion #11

Closed
erights opened this issue Sep 12, 2019 · 2 comments
Closed

Need safer expansion #11

erights opened this issue Sep 12, 2019 · 2 comments
Assignees
Labels
eventual-send package: eventual-send security

Comments

@erights
Copy link
Member

erights commented Sep 12, 2019

Even under SES, the currently proposed expansion of bob~.foo(carol) to

Promise.resolve(bob).post('foo', [carol])

is not actually safe against reentrancy. It would be if, under SES, all genuine promises were born frozen. However, SES has not specified this, and it is hard (possibly impossible) for a SES shim to enforce without an invasive rewrite. Given that genuine SES promises are not born frozen, someone could add a .post method override as an own property to a genuine bob promise before the above operation.

Instead,

  • the expansion should be in terms of calls to known functions, rather than late-bound calls to methods.
  • the spec should specify that the syntax expands to the original/internal form of these functions, so that even non-SES implementors of the spec have the safety property.

Possible expansions:

E.post(bob, 'foo', [carol])
E.get(bob, 'foo')
E.put(bob, 'foo', newValue)
E.delete(bob, 'foo')

If we do that, the analogy between these are corresponding Reflect methods becomes strong, so we should also adopt the Reflect terminology:

E.apply(bob, 'foo', [carol])
E.get(bob, 'foo')
E.set(bob, 'foo', newValue)
E.deleteProperty(bob, 'foo')

Given that E itself is frozen under SES, under SES this would already be a safe expansion. Only outside of SES is the divergence between the safety of the speced "original/internal" vs the expansion observable, which is fine.

Should E be global? Probably not. It should be exported from a js: standardized builtin module.

@warner warner transferred this issue from Agoric/eventual-send Dec 1, 2019
@warner warner added the eventual-send package: eventual-send label Dec 1, 2019
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
Other miscellaneous fixes.

closes Agoric#10
closes Agoric#11
closes Agoric#13
closes Agoric#14
closes Agoric#15
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
@erights
Copy link
Member Author

erights commented Feb 25, 2021

Hi @michaelfig I think you already fixed this and can close it out. So can I assign it to you?

@michaelfig
Copy link
Member

Hi @michaelfig I think you already fixed this and can close it out. So can I assign it to you?

Yes, this is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eventual-send package: eventual-send security
Projects
None yet
Development

No branches or pull requests

3 participants