Skip to content

1) Appending bytes to tx payload in data field scrambles e.g. displayed ERC20 transfer value & using sendTransaction to transfer ERC20s results in contract deployment #2744

@hilmarx

Description

@hilmarx

Describe the bug:

When sending transactions on Ethereum, e.g. an ERC transfer, you can append bytes to the encoded transfer payload. This is used by dApps to enrich the information of certain transactions with additional data. In our case, it is used to later verify on-chain that certain data which was appended to a regular ERC20 transfer was indeed signed by the user or not.

It can look something like this:

const erc20Interface = new ethers.utils.Interface(["function transfer(address recipient, uint256 amount)"])

const encodedData = erc20Interface.encodeFunctionData("transfer", [receiver, inputAmount])

const appendedData = new ethers.utils.AbiCoder().encode(
        ["address", "uint256"],
        [toCurrency, minimumReturn]
);

const concatData = encodedData + appendedData.substr(2)

const res = await provider.getSigner().sendTransaction({
        data: concatData,
        to: fromCurrency,
        gasLimit: 100000,
})

On desktop using Metamask or any other wallet, this works fine. However on Metamask mobile, the extra data which was appended to the regular erc20 encoded data seems to completely mess with the transaction, leading to the amount to be transferred to be completely messed up (Very dangerous) and the to field being undefined, resulting in the transaction trying to deploy a contract.

Screenshots

How it should look like:
IMG_1733

How it actually looks like:
IMG_1732

=> Sending this transaction will result in a contract being deployed which fails.

To Reproduce
Check out this codebase of using sendTransaction with a regular erc20 transfer payload (without appended data):

https://github.com/gelatodigital/sorbet-finance/blob/288c42f1dbb786d510265b95aad0cff164a47607/src/components/ExchangePage/index.jsx#L617-L625

You can try it out here:
https://sorbet-finance-git-fix-metamask-mobile-bug-gelato.vercel.app

It works fine, however defeats the purpose of the dApp (we need to append some additional payload to the data)

Now, check out this codebase, where we do append additional payload to the data field:
https://github.com/gelatodigital/sorbet-finance/blob/fe8e198d5d2d4c32d0a838b6b9e696c5bd04e496/src/components/ExchangePage/index.jsx#L617-L630

You can try it out here:
https://sorbet-finance-git-fix-metamask-mobile-bug-2-gelato.vercel.app

There you will see the bug occurs. As mentioned before, on Web Metamask and using other Wallets such as Formatic, it works fine. This issue also occurs on Mainnet, not only Matic.

Smartphone

  • Device: iPhoneX
  • OS: iOS14.4
  • App Version: v2.4.0

EDIT:

Replying to @MaartenWeber14 comment.

I just checked again and I think you are right, I am talking about two separate issues here.

Issue 1) If you append data to the e.g. transfer payload, the displayed values of how will be transferred get completely scrambled as seen in the screenshots of my original comment above.

Issue 2) If you use sendTransaction, even if you dont append any extra bytes to the payload, the to address is interpreted as undefined and a contract gets deployed instead of the to address being the ERC20 token.

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions