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

Wallet not returning correct selected address from eth_requestAccounts RPC call #30802

Closed
kdenhartog opened this issue Jun 4, 2023 · 3 comments · Fixed by brave/brave-core#18907
Assignees
Labels
feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. privacy QA Pass-Win64 QA/Test-Plan-Specified QA/Yes release-notes/include

Comments

@kdenhartog
Copy link
Member

kdenhartog commented Jun 4, 2023

From @MicahZoltu on discord

It appears that Brave is leaking account details to the dapp somehow. Using The Interceptor as described above (which interacts with Brave Wallet only through the installed window.ethereum object).

If I have 2 accounts in my Brave Wallet and I only allow the dapp to see the second one when prompted, somehow The Interceptor is seeing the first one. This should be impossible, and it means the dapp can also see the first account.

Second screenshot is The Interceptor prompting to allow the site to see an address, which appears after Brave Prompt. Interceptor gets the address from window.ethereum.request({method:'eth_requestAccounts'}) I believe, though it could also be from some event fired by Brave

image
image

@kdenhartog kdenhartog added privacy priority/P2 A bad problem. We might uplift this to the next planned release. feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop labels Jun 4, 2023
@onyb
Copy link
Member

onyb commented Jun 5, 2023

Here's a better way to reproduce it.

When site is not connected yet

  1. Open the console and enter the following:
     await window.ethereum.request({method:'eth_requestAccounts'})
  2. This pops up the connect panel. Make sure you select anything other than the first account.
  3. The promise in step 1 resolves to the address of the first account! 🐛
  4. Subsequent calls resolve to the correct address.

When site is already connected

Follow the same steps as above. The bug is not reproduced.

@kjozwiak
Copy link
Member

The above requires 1.53.102 or higher for 1.53.x verification 👍

@srirambv
Copy link
Contributor

Verification passed on

Brave 1.53.104 Chromium: 115.0.5790.40 (Official Build) (64-bit)
Revision 071c9ddea889c3c7887daf4eac13fed72d4fff62-refs/branch-heads/5790@{#979}
OS Windows 11 Version 22H2 (Build 22621.1848)
  • Verified steps from brave/brave-core#18907
  • Verified second account is shown when running await window.ethereum.request({method:'eth_requestAccounts'}) and selecting the second account in connect to Dapp prompt
30802.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. privacy QA Pass-Win64 QA/Test-Plan-Specified QA/Yes release-notes/include
Projects
Archived in project
6 participants