search

brave / brave-browser

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

Separate wallet parsing code into a separate utility process #27407

Open kdenhartog opened 1 year ago

kdenhartog commented 1 year ago

Currently all wallet code is being run through the browser process in a way that presents some security risks if untrusted inputs (e.g. HTTP responses or smart contract data) aren't properly parsed that leads to a bug. This issue is to address separating out wallet code that processes untrusted inputs into a separate utility process to reduce impact of a parsing bug.

kdenhartog commented 1 year ago

Some discussion occurred on this one on Slack and the outcome of the discussion sounded like doing this could lead to a major refactor. As a starting point to see the effectiveness of this a suggestion was made to figure out a list of top 10 parsing functions (or some other selected small list of functions) called so that we start with them and identify any performance trade offs made here.

Next steps for this one would be to identify some core parsing functions in the wallet that would be good candidates here.

kdenhartog commented 4 months ago

This likely needs to be turned into a meta issue to track various points of the code that could utilize this and track solving them individually. The overarching goal here is to use the rule of 2 as a defense in depth measure within the wallet, but the method for doing this may be different in certain cases.

For example, in https://github.com/brave/brave-core/pull/23730 the likely better option is to either move this into a rust based parser or even to move this to front-end code and handle parsing in JS at the expense of likely some performance benefits we're currently getting.

In any case, we now have the Brave Wallet utility process to handle this here which will help as we want to integrate it.