sherlock-audit / 2024-08-saffron-finance-judging

9 stars 5 forks source link

0xCarlos - call()` should be used instead of `transfer()` on an `address payable #95

Open sherlock-admin3 opened 2 months ago

sherlock-admin3 commented 2 months ago

0xCarlos

Medium

call()should be used instead oftransfer()on anaddress payable

Summary

Vulnerability Detail

Sending would fail if the fallback or receive function of msg.sender has a high gas consumption logic above 2300. Whether it's a contract or a multisg wallet.

function withdrawAmountVariablePending() public {
        uint256 amount = variableToPendingWithdrawalAmount[msg.sender];
        variableToPendingWithdrawalAmount[msg.sender] = 0;
        payable(msg.sender).transfer(amount);
    }

Impact

Using the transfer() function for an address will inevitably cause the transaction to fail when :

The requester's smart contract does not implement a payment function. The requester's smart contract implements a payable fallback that uses more than 2300 gas units. The requester's smart contract implements a payable fallback function that requires less than 2300 units of gas, but is called via proxy, increasing the call's gas usage to more than 2300.

In addition, the use of more than 2300 gas units may be mandatory for some multisig wallets.

Code Snippet

https://github.com/sherlock-audit/2024-08-saffron-finance/blob/38dd9c8436db341c331f1b14545770c1766fc0ee/lido-fiv/contracts/LidoVault.sol#L652-L657

Tool used

Manual Review

Recommendation

I recommend using call() instead of transfer().