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

Capability leak by fulfilling with a non-fresh handled presence #10

Closed
erights opened this issue Sep 22, 2019 · 10 comments
Closed

Capability leak by fulfilling with a non-fresh handled presence #10

erights opened this issue Sep 22, 2019 · 10 comments
Assignees
Labels
eventual-send package: eventual-send

Comments

@erights
Copy link
Member

erights commented Sep 22, 2019

Our way of fulfilling a handled promise with a fulfillment handler has a capability leak: Say Alice and Bob are within the same frozen root realms and supposedly isolated from each other. Alice creates a handled promise and then does Resolve(Object, fulfilledHandler), which fulfills it with Object. Bob then does Promise.resolve(Object) and retrieves Alice's handled promise.

Our presences are only safe when they are fresh. To enforce freshness, the presence should be created by the eventual-send package in the act of fulfilling a handled promise with a fulfilled handler. I don't see any way to do this while preserving the elegance of the current HandledPromise constructor.

API details aside, the most general way I can think of to ask the TCB to create a new fresh object, while still providing flexibility for what the object is, is to provide the parameters of Object.create and leave the creation up to the TCB.

Fortunately, there should be no motivated use case for simultaneously fulfilling a handled promise with a pre-existing object while also providing a fulfilled handler for that case. Fulfilled handlers should only be needed for fresh presences, so there shouldn't be any loss of needed functionality.

@erights erights changed the title Capability leak through current API for fulfilling with a handled presence Capability leak by fulfilling with a non-fresh handled presence Sep 22, 2019
@michaelfig
Copy link
Member

Can we do:

resolve(myObj, fulfilledhandler)
Innerresolve(Object.create(myObj))

The resolution would just be a fresh object whose prototype is myObj.

@erights
Copy link
Member Author

erights commented Sep 23, 2019

That would work and has some nice properties. But given the semantics of omitting the fulfilledHandler, it is surprising. Perhaps too surprising.

@michaelfig
Copy link
Member

Maybe the third argument to the executor should be resolvePresence, to distinguish it from standard resolve?

I misspoke once that this was already the case. Perhaps it should be that way.

@erights
Copy link
Member Author

erights commented Sep 23, 2019

I like that.

@erights
Copy link
Member Author

erights commented Sep 23, 2019

resolvePresence could return the new fresh presence that it made.

@erights
Copy link
Member Author

erights commented Sep 23, 2019

Should be called fulfillWithPresence. I propose the following parameters:

fulfillWithPresence(fulfilledHandler, 
                    optParent = Object.prototype, 
                    optDescriptors = undefined) -> presence

since only the fulfilledHandler should be mandatory. If the others are omitted, the returned presence is a fresh {}.

@erights
Copy link
Member Author

erights commented Sep 23, 2019

Should the fulfilledHandler be called the presenceHandler ?

@michaelfig
Copy link
Member

I like presenceHandler. Can I suggest that we just have:

fulfillWithPresence(presenceHandler) -> Object.create(null)

and so they can only mutate the presence, set the prototype or property descriptors if they have access to the Reflect API parts that can do that? That way we can include this constructor in Jessie without a privilege leak.

@erights
Copy link
Member Author

erights commented Sep 24, 2019

Nice! I hadn't thought about the Jessie implications. Yes.

@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
@michaelfig michaelfig self-assigned this 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
Other miscellaneous fixes.

closes Agoric#10
closes Agoric#11
closes Agoric#13
closes Agoric#14
closes Agoric#15
@michaelfig
Copy link
Member

This is implemented.

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

No branches or pull requests

3 participants