Yearn adapter doesn't take into consideration a potential loss when withdrawing from yearn

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/dcdd3ceda3d5bd87105e691ebc054fb8b04ae583/src/vault/adapter/abstracts/AdapterBase.sol#L173-L185 https://github.com/code-423n4/2023-01-popcorn/blob/dcdd3ceda3d5bd87105e691ebc054fb8b04ae583/src/vault/adapter/abstracts/AdapterBase.sol#L192-L204

Vulnerability details

Impact

A loss of as little of 1 "wei" will prevent a withdrawal/redeem transaction from executing.

Proof of Concept

Let's have a look step by step at what happens when withdrawing.

• User/contract calls withdraw(uint256 assets, address receiver,address owner).
• _previewWithdraw calculates the required amount of shares to burn to get the required number of assets.
• The shares are then converted to underlying shares to be withdrawn from yearn.
• The adapter withdraws the underlying shares from yearn vault to get assets.

Yearn vault might take funds from strategy to withdraw; it can happen with a loss; the loss is then deducted from the number of assets to transfer back to the adapter. The number of assets transferred from yearn will now be lower than the number of assets expected when calling withdraw(). The IERC20(asset()).safeTransfer(receiver, assets) will fail, funds will not be withdrawable.

The same issue happens when calling redeem().

Tools Used

Manual review

Recommended Mitigation Steps

Yearn returns the number of assets transferred when calling withdraw. This could be used to transfer the right number of assets. Please also note: Fixing this issue will not prevent withdrawal with a loss above 0.01% to fail. See the corresponding issue Funds might not be recoverable from vault blocking withdrawal and pause.

warning: When solving the issue, it's important to check for burned yearn vault shares. In a case where the vault doesn't have enough tokens to transfer back (because of some tokens locked in strategies), the yearn vault will not burn all the shares, contrary the popcorn vault will burn all the shares. The popcorn vault will have to adjust the burned amount or revert. A user could retry with a smaller amount.

[c4-judge]

dmvt marked the issue as primary issue

[c4-sponsor]

RedVeil marked the issue as disagree with severity

[c4-sponsor]

RedVeil marked the issue as sponsor confirmed

[c4-judge]

dmvt marked issue #581 as primary and marked this issue as a duplicate of 581

[c4-judge]

dmvt marked the issue as not a duplicate

[c4-judge]

dmvt marked the issue as nullified