`BeefyAdapter._protocolWithdraw()` can revert for some boosters

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/adapter/beefy/BeefyAdapter.sol#L210-L211 https://github.com/beefyfinance/beefy-contracts/blob/18ffce2c4c8f66865636efb92a24f7dc8f258e20/contracts/BIFI/vaults/BeefyVaultV6.sol#L143

Vulnerability details

When withdrawing from an adapter, the function does an internal call to _protocolWithdraw()

210:     function _withdraw(
211:         address caller,
212:         address receiver,
213:         address owner,
214:         uint256 assets,
215:         uint256 shares
216:     ) internal virtual override {
217:         if (caller != owner) {
218:             _spendAllowance(owner, caller, shares);
219:         }
220: 
221:         if (!paused()) {
222:             uint256 underlyingBalance_ = _underlyingBalance();  
223:             _protocolWithdraw(assets, shares);
224:             // Update the underlying balance to prevent inflation attacks
225:             underlyingBalance -= underlyingBalance_ - _underlyingBalance();
226:         }
227: 
228:         _burn(owner, shares);
229: 
230:         IERC20(asset()).safeTransfer(receiver, assets);
231: 
232:         harvest();
233: 
234:         emit Withdraw(caller, receiver, owner, assets, shares);
235:     }

This is the function in case of the BeefyAdapter

203:     function _protocolWithdraw(uint256, uint256 shares)
204:         internal
205:         virtual
206:         override
207:     {
208:         uint256 beefyShares = convertToUnderlyingShares(0, shares);
209:         if (address(beefyBooster) != address(0))
210:             beefyBooster.withdraw(beefyShares);
211:         beefyVault.withdraw(beefyShares);
212:     }

If there is a booster, the call will first withdraw beefyShares from beefyBooster, before withdrawing the same amount from the beefyVault.

The issue is that if a booster does not send back that exact amount of shares, the call to beefyVault will revert here as balanceOf(adapter) < beefyShares.

This can happen for a number of reason: (liquidity crunch, or a case where the booster charges a fee on withdrawal, keeping some of the shares).

In such case, withdrawals are essentially broken

Impact

Medium

Tools Used

Manual Analysis

Mitigation

Add a share balance check before and after the call to beefyBooster.withdraw(beefyShares), and use that difference as the input to beefyVault.withdraw()

[c4-sponsor]

RedVeil marked the issue as sponsor disputed

[c4-judge]

dmvt marked the issue as unsatisfactory: Invalid

[dmvt]

From the sponsor:

the booster contract returns assets 1:1 and there cant be any liquidity crunch or withdrawalFee nor slippage so its basically not possible that the booster returns smth else than the exact amount of assets we are asking for