Description

The value of queuedWithdrawalTvl can be artificially inflated, which might revert the transactions calling fetchDeployAmounts or deployPosition in StrategyVault and _borrow in the HookAaveFixYield and HookAave contracts.

When a user calls requestWithdrawal, the value of totalQueuedShares[deployId] is increased by the amount of shares. The user can then transfer their funds to another wallet and call requestWithdrawal again, which would increase the totalQueuedShares[deployId] for the second time.

This can be repeated multiple times to artificially increase the value of totalQueuedShares[deployId]. When the owner closes this position using closePosition, this value will be added to queuedWithdrawalTvl, thus increasing its value more than intended.

If the value of queuedWithdrawalTvl becomes greater than totalAssets() after a successful exploit, it will revert the function call fetchDeployAmounts and deployPosition in the StrategyVault contract due to integer underflow. This would also revert any call to availableUnderlying, which is called in _borrow in the hook contract.

Impact

Certain function calls would revert, and new positions cannot be deployed.

Recommendations

When a user requests withdrawal using the requestWithdrawal, these funds should not be allowed to be transferred to other wallets.

An additional check can be implemented in the _transfer function that checks that no more than balanceOf[sender] - withdrawQueue[sender].shares are transferred from the sender's address.

    function _transfer(address sender, address recipient, uint256 amount) internal virtual override {
+        require(balanceOf[sender] - withdrawQueue[sender].shares > amount,"Not enough funds");
        _updateUserEmissions(sender,amount,false);
        _updateUserEmissions(recipient,amount,true);
        super._transfer(sender, recipient, amount);
    }

Remediation

The issue was fixed in commit 11f6797↗.