MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
12.05k stars 4.92k forks source link

Trezor feature persists previous trezor Xpub #5781

Open danfinlay opened 5 years ago

danfinlay commented 5 years ago

If I have two trezors, with two different mnemonics Then connect trezor A, use it. Connect trezor B, select "connect hardware wallet",

MetaMask shows me the old Trezor's account list.

brunobar79 commented 5 years ago

@danfinlay wow. Good find!

brunobar79 commented 5 years ago

Ok, so the deal is that we're caching the XPUB unless you click Forget this device

screen shot 2018-11-20 at 9 25 49 pm

I have no way of making sure the device is different / same unless I go to the device and ask for the xpub key, which means there's gonna be a popup from Trezor, asking for your approval.

I guess we have to make the decision of: Should we stop caching the XPUB and ask for confirmation every time you plug in your wallet?

This is definitely more secure and will solve the issue that you reported but:

An important point is that there's already a workaround which is to use the "Forget this device" feature, which will prevent the issue.

Maybe the solution it's just to add better info about how the feature works and what the "Forget this device" feature does.

WDYT?

cc: @bdresser @cjeria

bdresser commented 5 years ago

Hey, four months later :)

I think the double-trezor case is rare enough that we shouldn't change the basic flow to solve this. Going to move to icebox and see if this comes up again.

We could always add a disclaimer towards the bottom like "seeing stale data? forget this device and reconnect"

tmashuang commented 3 years ago

The initial implementation took into account caching but I personally agree with that clicking Connect with a HW wallet would ideally act like a new connection everytime.