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

9 stars 5 forks source link

bumble - Pending withdraw amounts cannot be claimed by inadequate smart contract #98

Open sherlock-admin3 opened 2 months ago

sherlock-admin3 commented 2 months ago

bumble

Medium

Pending withdraw amounts cannot be claimed by inadequate smart contract

Lines of code:

Vulnerability details

The use of transfer() function for an address will make the transaction fail when the claimer is:

Issue inspired by https://solodit.xyz/issues/m-01-call-should-be-used-instead-of-transfer-on-an-address-payable-code4rena-backd-backd-contest-git

Impact

Smart contracts with one of the conditions above calling their variable side pending withdraws (withdrawAmountVariablePending) will have their transaction revert, having their pending funds locked funds in the contract.

  /// @notice withdrawal of funds for Variable side
  function withdrawAmountVariablePending() public {
    uint256 amount = variableToPendingWithdrawalAmount[msg.sender];
    variableToPendingWithdrawalAmount[msg.sender] = 0;
@>    payable(msg.sender).transfer(amount);
  }

Another issue caused by the same root is when calling finalizeVaultOngoingVariableWithdrawals:

 function finalizeVaultOngoingVariableWithdrawals() external {
    uint256[] memory requestIds = variableToVaultOngoingWithdrawalRequestIds[msg.sender];
    if(variableToPendingWithdrawalAmount[msg.sender] != 0) {
@>      withdrawAmountVariablePending();
      if(requestIds.length == 0) {
        return;
      }
    }
...

When it first checks if the user has pending variable withdraws, causing the transaction to fail, not following up with the function logic.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Use call instead