sherlock-audit / 2023-01-derby-judging

4 stars 1 forks source link

ivanrdy - Deposits in providers can fail on zero amount transfers if amount is set to zero #384

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ivanrdy

false

Deposits in providers can fail on zero amount transfers if amount is set to zero

Deposits in providers can fail on zero amount transfers if amount is set to zero

Amount can be zero, while safetransferfrom do attempt to send it in such a case anyway as there is no check in place. Some ERC20 tokens do not allow zero value transfers, reverting such attempts.

So, a combination of zero amount and such a token will revert any deposit operations, rendering deposit functionality unavailable

Proof of Concept: Amount can be set to zero: https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/YearnProvider.sol#L19 https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/TruefiProvider.sol#L19 https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/AaveProvider.sol#L20

Amount transfer attempts are now done uncoditionally in all the providers.

YearnProvider, TruefiProvider, AaveProvider do not check the amount to be send, (_amount) , before transferring: https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/YearnProvider.sol#L26 https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/TruefiProvider.sol#L26 https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/AaveProvider.sol#L27

References Some ERC20 tokens revert on zero value transfers: https://github.com/d-xo/weird-erc20#revert-on-zero-value-transfers

No tool was used Manual Review

Recommendation: Consider checking the amount parametr and do transfer only when it is positive.