USER WILL SEND TRANSACTION GAS WHICH IS ONLY ENOUGH TO EXECUTE `StakedUSDeV2.unstake` FUNCTION SUCCESFULLY BUT NOT ENOUGH TO FULLY EXECUTE THE `silo.withdraw` THUS LOSING ALL USER FUNDS

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/USDeSilo.sol#L28-L30 https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDeV2.sol#L82-L86

Vulnerability details

Impact

The StakedUSDeV2.unstake function is used to claim the staking amount after the cooldown period has finished. The unstake function will reset the userCooldown.cooldownEnd and userCooldown.underlyingAmount parameters to 0 for the msg.sender once the cool down periods has elapsed and then will withdraw the underlying assets from the silo contract to the receiver, as shown below:

if (block.timestamp >= userCooldown.cooldownEnd) {
  userCooldown.cooldownEnd = 0;
  userCooldown.underlyingAmount = 0;

  silo.withdraw(receiver, assets);

The silo.withdraw function will transfer the underlying assets to the receiver address using the ERC20.transfer function. But the issue here is that the silo.withdraw function does not revert during transfer function failure and neither does silo.withdraw return a boolean value indicating the success of the fund transfer to the calling StakedUSDeV2.unstake function.

The issue here is if the msg.sender unknowingly or mistakenly passes in a transaction gas amount which is just enough to execute the StakedUSDeV2.unstake function but does not have enough gas to fully complete the silo.withdraw external function execution, the owner of the underlying funds will lose all of his underlying funds. This is because the transaction did not have enough gas to transfer the funds to the receiver address since the silo.withdraw did not execute completely due to running out of transaction gas. But the 1/64th amount of gas was available to the calling function to complete the StakedUSDeV2.unstake thus completing the transaction and setting the userCooldown.cooldownEnd and userCooldown.underlyingAmount values to 0 thus making the fund owner to lose all of his underlying funds in the silo contract.

Proof of Concept

  function withdraw(address to, uint256 amount) external onlyStakingVault {
    USDE.transfer(to, amount);
  }

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/USDeSilo.sol#L28-L30

    if (block.timestamp >= userCooldown.cooldownEnd) {
      userCooldown.cooldownEnd = 0;
      userCooldown.underlyingAmount = 0;

      silo.withdraw(receiver, assets);

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDeV2.sol#L82-L86

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to update the silo.withdraw function to return a boolean success value to the StakedUSDeV2.unstake function to indicate the successful execution of the silo.withdraw transaction. This boolean value should be evaluated in the StakedUSDeV2.unstake function and if the boolean value is false the transaction should revert.

The recommended modification changes to the code snippets are shown below:

USDeSilo.withdraw function :

  function withdraw(address to, uint256 amount) external onlyStakingVault returns(bool) {

    bool success = USDE.transfer(to, amount);
    require(success, "Fund transfer failed");

    return true;
  }

StakedUSDeV2.unstake function :

  function unstake(address receiver) external {
    UserCooldown storage userCooldown = cooldowns[msg.sender];
    uint256 assets = userCooldown.underlyingAmount;

    if (block.timestamp >= userCooldown.cooldownEnd) {
      userCooldown.cooldownEnd = 0;
      userCooldown.underlyingAmount = 0;

      bool success = silo.withdraw(receiver, assets);
      require(success, "Withdraw transaction failed");

    } else {
      revert InvalidCooldown();
    }
  }

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #73

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid