code-423n4 / 2022-12-tigris-findings

8 stars 4 forks source link

Unsafe ERC20 methods #557

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/StableVault.sol#L46-L50 https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/StableVault.sol#L68-L71

Vulnerability details

Impact

The ERC20.transfer() and ERC20.transferFrom() functions return a boolean value indicating success. This parameter needs to be checked for success. USDT, one of the allowed tokens, does not revert if the transfer failed but returns false instead.

Proof of Concept

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/StableVault.sol#L46-L50

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/StableVault.sol#L68-L71

Tools Used

Manual review.

Recommended Mitigation Steps

Recommend using OpenZeppelin's SafeERC20 versions with the safeTransfer and safeTransferFrom functions that handle the return value check as well as non-standard-compliant tokens.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #198

c4-judge commented 1 year ago

GalloDaSballo marked the issue as not a duplicate

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #352

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Out of scope