Jeiwan - Liquidity providers can lose funds when a withdraw proxy is not set for an epoch

[sherlock-admin]

Jeiwan

medium

Liquidity providers can lose funds when a withdraw proxy is not set for an epoch

Summary

Liquidity providers can lose funds when a withdraw proxy is not set for an epoch

Vulnerability Detail

The transferWithdrawReserve function of PublicVault sends withdrawal reserves to a WithdrawProxy (PublicVault.sol#L341) and subtracts transferred amount from the reserves. However, if a withdraw proxy is not set for an epoch, there's a false positive: withdrawReserve is updated but no funds are actually transferred:

function transferWithdrawReserve() public {
  // check the available balance to be withdrawn
  uint256 withdraw = ERC20(underlying()).balanceOf(address(this));
  emit TransferWithdraw(withdraw, withdrawReserve);

  // prevent transfer of more assets then are available
  if (withdrawReserve <= withdraw) {
    withdraw = withdrawReserve;
    withdrawReserve = 0;
  } else {
    withdrawReserve -= withdraw;
  }
  emit TransferWithdraw(withdraw, withdrawReserve);

  address currentWithdrawProxy = withdrawProxies[currentEpoch - 1]; //
  // prevents transfer to a non-existent WithdrawProxy
  // withdrawProxies are indexed by the epoch where they're deployed
  if (currentWithdrawProxy != address(0)) { // @audit false positive: transferring is skipped silently
    ERC20(underlying()).safeTransfer(currentWithdrawProxy, withdraw);
    emit WithdrawReserveTransferred(withdraw);
  }
}

Impact

Liquidity providers might not be able to withdraw liquidity they requested because it wasn't transferred to a WithdrawProxy due to a mistake, yet accounting was updated.

Code Snippet

See Vulnerability Detail

Tool used

Manual Review

Recommendation

Consider reverting in the case when no withdraw proxy is set for the current epoch.

Duplicate of #163

[SantiagoGregory]

We now check that the withdrawProxy for the current epoch (technically previous epoch) exists before transferring.

[IAmTurnipBoy]

Escalate for 1 USDC

Duplicate of #163, which I believe is also invalid

[sherlock-admin]

Escalate for 1 USDC

Duplicate of #163, which I believe is also invalid

You've created a valid escalation for 1 USDC!

To remove the escalation from consideration: Delete your comment. To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[Evert0x]

Escalation accepted. Same issue with different recommendation

[sherlock-admin]

Escalation accepted. Same issue with different recommendation

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.