0xsequence / erc-1155

Ethereum Semi Fungible Standard (ERC-1155)
https://sequence.build
Other
320 stars 119 forks source link

H1 - GasReceipt injection using `transferData` #24

Open PhABC opened 4 years ago

PhABC commented 4 years ago

image

by @Agusx1211

PhABC commented 4 years ago

I'm hesitant with this suggested fix, because when one assumes that a malicious dapp can make sign arbitrary transfer data, then the amount of harm they can do goes beyond gas fee reimbursement. For instance, a dapp could make you sell X for tokens Y on Niftyswap and transfer to themselves tokens Y instead of you receiving them.

Imo it should be the users and wallets responsibility to make sure that a user is aware of all the data that is signed such that nothing unintentional is included, where gasReceipt is just one example of things that the users need to verify.

Agusx1211 commented 4 years ago

The way I see it, transferData is not expected to be able to manipulate the funds of the users, so any security mechanism designed to protect the user from a malicious dApp (UI, Audit, etc.) may oversee the possibility of some party injecting transferData.

It also should be considered that this injection can also be performed in reverse, meaning that a relayer could pass the gasReceipt as additional transferData, this could have unforeseen consequences that are hard to predict, and it would depend on the receiving contract. Still, it could open the door to attacks in the future.