sherlock-audit / 2023-06-Index-judging

6 stars 2 forks source link

0xcc - Token transfers do not verify that the tokens were successfully transferred. #64

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

0xcc

medium

Token transfers do not verify that the tokens were successfully transferred.

Summary

Some ERC20 tokens don’t throw but just return false when a transfer fails. This implies that not all tokens can be used

Vulnerability Detail

Using token.transferFrom(msg.sender, addr, amount); is NOT enough. The risk is that you're assuming the transfer was successful when actually it wasn't. Example of tokens that don't revert: ZRX, HT, WOO. Also using require(token.transferFrom(msg.sender, address(0),token amount), "..."); isn't quite safe. There are indeed some common tokens that aren't ERC20-compliant and don't return true on success. USDT is an example.

Impact

This can be abused to trick the _executeBid() function to receive the component amount without providing any tokens.

Code Snippet

https://github.com/sherlock-audit/2023-06-Index/blob/main/index-protocol/contracts/protocol/modules/v1/AuctionRebalanceModuleV1.sol#L940

Tool used

Manual Review

Recommendation

The safest method is using this:

(bool success, bytes memory data) = address(token).call(abi.encodeWithSelector(token.transferFrom.selector, from, to, value));
require(success && (data.length == 0 || abi.decode(data, (bool))), 'token transfer from sender failed');