MetaMask / metamask-extension

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

Ledgerjs is likely not the latest version making it impossible to verify amount and destination address on some ERC-20 tokens with Ledger #8875

Closed noops888 closed 3 years ago

noops888 commented 4 years ago

Describe the bug Some ERC-20 tokens do not show the amount being sent or the destination address on the ledger. Instead they show "data present" and the contract address instead of the amount of the token being sent and the destination address. This greatly reduces hardware wallet security.

Two tokens that currently exhibit this problem are XAUt and DAI but there are likely others.

To Reproduce (REQUIRED) Steps to reproduce the behavior, libraries used with version number, and/or any setup information to easily reproduce:

  1. Connect a ledger nano S to metamask.
  2. Attempt to send XAUt or DAI and carefully observe the ledger nano s screen and note that you will not see the amount of the token being sent or the destination address.

Expected behavior USDT is an example of a token that works as expected, you see the amount and the destination address shown on the ledger screen.

Screenshots

Browser details (please complete the following information):

Additional context (Error Messages, etc.)

Ledgers developers suggest this problem is caused because MetaMask is not using the latest version of "ledgerjs"

ciconsulting commented 3 years ago

This is also the case for CGT (0xf5238462e7235c7b62811567e63dd17d12c2eaa0) with Ledger Nano S and Ledger Nano X + MetaMask. Instead of the destination address the contract address is shown. The max fees are shown but the amount being sent is not shown. CGT has been fully supported by Ledger Nano S, Nano X and Ledger Live desktop software for months.

mudgen commented 3 years ago

It seems like this issue might be fixed if Metamask merges this pull request: https://github.com/MetaMask/eth-ledger-bridge-keyring/pull/45

Gudahtt commented 3 years ago

I was able to verify the described behavior; USDT and USDC show the amount and destination address on the Ledger Nano S, but DAI does not.

When I tested this with https://github.com/MetaMask/eth-ledger-bridge-keyring/pull/53 (the successor PR to https://github.com/MetaMask/eth-ledger-bridge-keyring/pull/45), the amount and destination address was shown even with DAI.

Gudahtt commented 3 years ago

https://github.com/MetaMask/eth-ledger-bridge-keyring/pull/53 has been merged, and I've just tested this using the the latest production MetaMask version on Chrome (v9.0.1). When confirming a DAI transfer, it now correctly shows the amount and destination address, just as it did for USDC and USDT.

metal450 commented 3 years ago

This is still happening. I'm observing it for the Everest token: https://coinmarketcap.com/currencies/everest/