code-423n4 / 2021-06-gro-findings

0 stars 1 forks source link

`calcProtocolWithdraw` withdraws more than necessary? #109

Closed code423n4 closed 2 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

Vulnerability Details

The Allocation.calcProtocolWithdraw function computes how much can be withdrawn per strategy. At the end of the function, if protocolWithdrawalUsd[i], the difference of current and target allocation, is less than the swap in amount, the withdrawal is set to the swap in amount.

if (protocolWithdrawalUsd[i] > 0 && protocolWithdrawalUsd[i] < allState.stableState.swapInAmountsUsd[i]) {
    // if we would swap in more than we'd withdraw, just "withdraw" the swap in amount
    // why not both? 🤷‍♂️
    protocolWithdrawalUsd[i] = allState.stableState.swapInAmountsUsd[i];
}

But it could even be set to the difference plus the withdrawal amount and it'd still hit the target amount?

Impact

Less efficient withdrawals

Recommended Mitigation Steps

Should it not always add allState.stableState.swapInAmountsUsd[i] to protocolWithdrawalUsd[i]?

kitty-the-kat commented 2 years ago

It is enough to withdraw the max(protocolWithdrawalUsd[i], allState.stableState.swapInAmountsUsd[i]). Withdrawal from over-exposed protocol can impact stablecoin exposure and protocol exposure at that same time.

ghoul-sol commented 2 years ago

Going to trust sponsor here. Invalid.