search

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

0 stars 0 forks source link

Yearn vault withdrawals in redeems will always fail leading to lock/loss of user deposits #72

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

The _withdrawFromVault() calculates the token balance of contract before withdrawal and saves it in previousBalance. It then withdraws from the Yearn vault and calculates the token balance after withdrawal to save it in currentBalance. So currentBalance should be > previousBalance if >0 tokens are withdrawn from the vault.

However, it incorrectly interchanges use of previousBalance and currentBalance in calculating the tokens withdrawn from the vault with the previousBalance.sub(currentBalance) calculation which will always cause _withdrawFromVault() to revert because currentBalance > previousBalance if/when >0 tokens are withdrawn from vault.

Impact: This means that tokens can be supplied to the vault but never redeemed leading to lock/loss of user deposits.

Proof of Concept

https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/yield-source/YearnV2YieldSource.sol#L194

https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/yield-source/YearnV2YieldSource.sol#L185-L192

Tools Used

Manual Analysis

Recommended Mitigation Steps

Switch the use of previousBalance and currentBalance and change the code to currentBalance.sub(previousBalance) on Line 194.

asselstine commented 3 years ago

See https://github.com/code-423n4/2021-06-pooltogether-findings/issues/90

dmvt commented 3 years ago

duplicate of #90