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

5 stars 2 forks source link

Not using safe transfer #448

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/rubiconPools/BathToken.sol#L605 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L565 https://github.com/RubiconDeFi/rubicon-protocol-v1/blob/6a0e1d89ff710f81428228d132e7d0c42de3a3cd/contracts/rubiconPools/BathPair.sol#L601 https://github.com/RubiconDeFi/rubicon-protocol-v1/blob/6a0e1d89ff710f81428228d132e7d0c42de3a3cd/contracts/rubiconPools/BathPair.sol#L615

Vulnerability details

ERC20 missing return value check

Impact

Funds may not be transferred but accounted as so

Proof of Concept

The erc20 interface ensures a token will return false on transfer failure but not necessarily revert. The return value should always be checked to account for these types of tokens.

This happens in various instances : https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L605

https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L565

https://github.com/RubiconDeFi/rubicon-protocol-v1/blob/6a0e1d89ff710f81428228d132e7d0c42de3a3cd/contracts/rubiconPools/BathPair.sol#L601

https://github.com/RubiconDeFi/rubicon-protocol-v1/blob/6a0e1d89ff710f81428228d132e7d0c42de3a3cd/contracts/rubiconPools/BathPair.sol#L615

Also all token transfers in rubiconRouter

Recommended Mitigation Steps

Use SafeERC20 library from openZeppelin

bghughes commented 2 years ago

Duplicate of #316