code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

when calling transferFrom or transfer of an external token a bool indicating success / fail is returned. Not checking this return value can cause the transaction to proceed even when the transfer has failed.

for example in BathToken._withdraw token.transfer can fail for some reason and return false, but the withdrawal will succeed.

Proof Of Concept

From the ERC20 docs:

transfer(address recipient, uint256 amount) → bool external Moves amount tokens from the caller’s account to recipient.

Returns a boolean value indicating whether the operation succeeded.

Emits a Transfer event.

and same for transferFrom and approve.

An example for such asset is the Basic Attention Token which implements this transfer() and this transferFrom()

Tools Used

Manual Inspection with VSCode.

Recommended Mitigation Steps

check the return values of these calls: token.transferFrom(msg.sender, address(this), amount); -> require(token.transferFrom(msg.sender, address(this), amount), "transfer failed");

and the same for transfer()

or use the safe versions of these function which checks the return value

KenzoAgada commented 2 years ago

Duplicate of #316.

bghughes commented 2 years ago

Duplicate of #316

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