sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

Stoicov - Solmate's safeTransferLib does not check if a token address has associated code with it, which may cause loss of funds. #75

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

Stoicov

false

Solmate's safeTransferLib does not check if a token address has associated code with it, which may cause loss of funds.

Summary

The lack of contract existence check upon a transfer can lead to messed accounting and loss of funds.

Vulnerability Detail

There is a key difference between the implementations of OZ's safeERC20 and Solmate's safeTransferLib. The latter does not check if the token address has associated code with it. This responsibility is delegated to the caller as stated in the NatSpec documentation of Solmate's safeTransferLib. This can mess up the accounting due to the fact that safeTransfer and safeTransferFrom won't revert if a non-existent token is passed. Consider the following:

The GOVERNOR has the ability to set rewardsToken in Factory. Because of the following check address(rewardsToken) == address(0)); it is made clear that this token can be set only once. There are no checks implemented if the token address actually contains code so what can happen is that the GOVERNOR , either by mistake or willingly, can set rewardsToken to an address that has no code within it. This will DOS the protocol because as mentioned above rewardsToken can be set only once and although it is not actually a token transfers will succeed but no tokens are being transferred in reality.

Impact

Lenders have no incentive to give loans.

Code Snippet

https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/Factory.sol#L272-L274

Tool used

Manual Review

Recommendation

Prior to safeTransfer or safeTransferFrom add code existence checks. Also check the balance before the transfer and after to ensure tokens are actually being transfered.

Duplicate of #82

sherlock-admin2 commented 1 year ago

2 comment(s) were left on this issue during the judging contest.

panprog commented:

low, because it's admin mistake to set reward rate long before setting the reward token or set invalid reward token address

MohammedRizwan commented:

invalid issue as rewardToken will be set by timelock governor