search

code-423n4 / 2022-05-rubicon-findings

5 stars 2 forks source link

Unchecked ERC20 return values #433

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L202 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L274 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L366 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L419 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L251 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L406 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L377 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L320 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L303 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L251

Vulnerability details

The return values of ERC20 transfer and transferFrom are not consistently checked throughout the codebase. Tokens that return false rather than revert to indicate failed transfers may silently fail rather than reverting as expected.

In addition, since ERC20 is used directly for many transfers (rather than a safe transfer helper), transferring weird ERC20s with missing return values may revert in unexpected cases.

-RubiconRouter#swap -RubiconRouter#swapEntireBalance -RubiconRouter#buyAllAmountForETH -RubiconRouter#offerForETH -RubiconRouter#_swap -RubiconRouter#offerWithETH -RubiconRouter#buyAllAmoutForETH -RubiconRouter#maxSellAllAmount -RubiconRouter#maxBuyAllAmount -RubiconRouter#_swap

Recommendation: Use a safe transfer library like OpenZeppelin SafeERC20 to ensure consistent handling of ERC20 return values and abstract over inconsistent ERC20 implementations.

bghughes commented 2 years ago

Duplicate of #316