mycelium-com / wallet-android

Mycelium Bitcoin Wallet for Android
http://mycelium.com
Other
660 stars 320 forks source link

Trezor change address #509

Closed d3987ef8 closed 5 years ago

d3987ef8 commented 5 years ago

Sometimes you need to confirm the change address with Trezor.

Given there is no way to check the change address is correct, and that the change address represents everything remaining in the wallet, having the requirement to confirm the change address is a bug that could lead to a significant loss of funds.

Most of the time there is no need to confirm the change address, and the Trezor automatically detects it.

Giszmo commented 5 years ago

Hi. Thank you for reporting this issue. We will see if we can better explain what's going on. The screen is already explaining it but the crucial point is that the Trezor is displaying the change output as a regular output to a different account on the same Trezor. It shows "Account 1" or something, so you know the change is not leaving the device.

d3987ef8 commented 5 years ago

Ah, thanks, I didn't know that.

This shouldn't be tagged as a question, it's still a bug. If someone is MITM and got a malicious version of mycelium app, most users are going to get fucked by this message and would transfer nearly 100% of their account to some hacker's address thinking it's the change address. They will accept that behaviour if they have been using Mycelium for years and it feels like a normal process for them.

I understand you're saying it's possible to distinguish between a hacker's output and the regular change output on the Trezor. I can't verify this right now but assuming it's correct, you need to put the information how to distinguish on the app where it says "output to a change address", or you need to make sure it always uses a change address that Trezor detects and doesn't prompt a confirmation for.

Actually to be honest I don't understand why Trezor even need a confirmation for a transfer to an address that is easily detectable to be on the Trezor. I'll email them about it.

Recommend to tag this as "nice to have" or "help wanted" if you cbf fixing it, if nobody touches it for a year I'll submit a PR amending the message.

Giszmo commented 5 years ago

Trezor is aware of this and were not very welcoming about our more private way of handling those new standards. I just explained it on reddit.

d3987ef8 commented 5 years ago

Am I understanding correctly that if Trezor doesn't support the new standard, you send the change to a change address in Mycelium and move the funds off the Trezor?

That can't be right, I'm sure I'm misunderstanding the problem here.

I'm not 100% sure of the problem but it sounds like a good idea in the meantime is to simply disable the new standards for Trezor accounts until Trezor supports the new standard.