search

code-423n4 / 2023-06-stader-findings

1 stars 1 forks source link

Using transferFrom on ERC20 tokens transfer #368

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/Auction.sol#L55 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/SDCollateral.sol#L47

Vulnerability details

Impact

The transferFrom() method is used instead of safeTransferFrom which is not recommended. OpenZeppelin’s documentation discourages the use of transferFrom(), use safeTransferFrom() whenever possible Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)'s transfer() and transferFrom() functions on L1 do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert.

Proof of Concept

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/Auction.sol#L55 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/SDCollateral.sol#L47

Tools Used

Manual Review

Recommended Mitigation Steps

Use OpenZeppelin’s SafeERC20's safeTransferFrom over transferFrom when exercising.

Assessed type

ERC20

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #380

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid