Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Pass wallet API in as parameter, not as global #53

Open
danfinlay opened this issue Oct 7, 2019 · 3 comments
Open

Pass wallet API in as parameter, not as global #53

danfinlay opened this issue Oct 7, 2019 · 3 comments
Labels
security Security-related

Comments

@danfinlay
Copy link
Collaborator

By exposing the wallet API as a global variable, we expose it (and the plugin's restricted methods, which may be sensitive) to not just the plugin's entry point, but all of its dependencies, making plugins more prone to dependency supply-chain attacks.

Instead, we could require plugins export an object or function, which would allow us to pass in authority in a more controlled way. For example, a plugin may look more like:

module.exports = function (wallet) {
  wallet.onUnlock(() => console.log('tada!'))
}

We could also have plugins export an object, which would allow it to register methods, but this opens it to potential name collisions, I'm not sure which API is better, but am open to reasoned opinions.

@danfinlay danfinlay added PROD-BLOCKER Issues that need to be resolved before this branch would ever be eligible for production. security Security-related labels Oct 7, 2019
@bitpshr
Copy link
Contributor

bitpshr commented Oct 21, 2019

Note: this should include updating examples.

@bitpshr bitpshr self-assigned this Oct 21, 2019
@danfinlay
Copy link
Collaborator Author

@rekmarks rekmarks assigned rekmarks and unassigned bitpshr Oct 31, 2019
@danfinlay danfinlay removed the PROD-BLOCKER Issues that need to be resolved before this branch would ever be eligible for production. label Nov 12, 2019
@danfinlay
Copy link
Collaborator Author

It is unclear whether the proposed approach had the benefit of mitigating the suggested attack. I have removed the blocker tag until we can further investigate whether there is a safe way to expose an object to a SES-confined script in a way that its dependencies don't necessarily also have global access to that object.

(The answer may simply be that the plugins would want to run SES around their dependencies, in which case it is beyond our control, but #98 might make it easier to do that).

I'd love to get input from @erights on whether this goal should be pursued by our plugin system itself, or just be a best practice for plugin developers to be encouraged towards.

@rekmarks rekmarks removed their assignment Nov 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
security Security-related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants