brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.51k stars 2.26k forks source link

audit and sandbox npm dependencies in brave wallet UI #20724

Open diracdeltas opened 2 years ago

diracdeltas commented 2 years ago

https://github.com/brave/brave-core/tree/master/components/brave_wallet_ui depends on a number of npm packages, which often have security vulnerabilities. we should sandbox these as much as possible so that a vulnerability in any of these dependencies (or their subdependencies) cannot access a user's private wallet key or do transactions on their behalf.

some dependencies like trezor-connect can be put into a chrome-untrusted frame without affecting functionality: https://source.chromium.org/chromium/chromium/src/+/main:docs/chrome_untrusted.md?q=chrome-untrusted&ss=chromium. for ones that can't, we should consider using https://github.com/LavaMoat/LavaMoat so that each dependency only has access to the minimum necessary privileges.

diracdeltas commented 10 months ago

@josheleonard FYI we are already doing the chrome-untrusted approach when possible. i would suggest tackling this by using https://metamask.io/news/security/using-lavamoat-to-solve-software-supply-chain-security/ and looking at what metamask is doing these days for sandboxing deps (might have changed since i opened this issue)

josheleonard commented 10 months ago

@diracdeltas, I started looking into lavamoat usage for dependency isolation. When running lavamoat-node against brave_wallet_page.bundle.js an error is thrown due to one of Recharts' dependencies (react-smooth) modifies Number.isInfinite ("primordial mutation")

My hope is that isolating this dependency to an untrusted-iframe will allow us to to move forward towards implementing lavamoat, since the dep should no longer be included in the main js bundle once it is isolated 🤞

josheleonard commented 10 months ago

Needing a webpack upgrade to v5 in order to utilize lavamoat Update webpack to v5 (or later)