Lines of code
<https://github.com/code-423n4/2022-02-redacted-cartel/blob/main/contracts/BribeVault.sol#L296-L297&gt;
<https://github.com/code-423n4/2022-02-redacted-cartel/blob/main/contracts/ThecosomataETH.sol#L146&gt;
<https://github.com/code-423n4/2022-02-redacted-cartel/blob/main/contracts/ThecosomataETH.sol#L164&gt;

Vulnerability details

Impact

There are multiple external calls to IERC20.transfer() that suppose to transfer out tokens from the system. However, there are tokens like USDT that don’t return any return value in their implementation of the transfer() function, so calling IERC20.transfer() on them will cause the transaction a revert (a kind of DoS - the funds are stuck at the contract).

In addition, tokens which do return a return value in their implementation of the transfer() function should return whether the transfer succeeded of failed, but that return value is never tested by the calling contract. This can lead to false events (for example withdrawal that didn’t actually work) or worse - the system can enter an invalid state, for example wrong rewards update of the Reward Distributor after a failed call to IERC20.transfer()

Proof of Concept

For example: the admin can’t withdraw tokens like USDT in BribeVault.emergencyWithdrawERC20() and in ThecosomataETH.withdraw(), and he/she can’t charge fees in BribeVault.transferBribes().

Another example, as mentioned before: the call to IERC20.transfer in BribeVault.transferBribes() may failed and return false, but there will still be a call to IRewardDistributor(distributor).updateRewardsMetadata(distributions), which can enter the system into a bad state because there wasn’t any actual transfer.

Tools Used

Manual code review.

Recommended Mitigation Steps

Use OpenZeppelin’s SafeERC20.safeTransfer() function instead their IERC20.transfer() function.

The text was updated successfully, but these errors were encountered:

All reactions