Summary
The UpliftOnlyExample hook contract fails to properly implement the onBeforeRemoveLiquidity function, causing all liquidity removal operations to revert when shouldCallBeforeRemoveLiquidity is enabled. This creates a denial of service condition where users cannot remove their liquidity from affected pools.
Vulnerability Details
In the Vault.sol contract, when removing liquidity, the vault checks if shouldCallBeforeRemoveLiquidity() is true. If enabled, it calls the hook's callBeforeRemoveLiquidityHook function which returns false by default since onBeforeRemoveLiquidity is not implemented in the UpliftOnlyExample contract.
This differs from liquidity addition where the hook's onBeforeAddLiquidity function is properly implemented to return true, allowing the operation to proceed.
The relevant code here shows that for removing liquidity it will revert since there is no override of the onBeforeRemoveLiquidity function.
if (poolData.poolConfigBits.shouldCallBeforeRemoveLiquidity()) {
HooksConfigLib.callBeforeRemoveLiquidityHook(
minAmountsOutScaled18,
msg.sender,
params,
poolData,
_hooksContracts[params.pool]
);
}
function callBeforeRemoveLiquidityHook(
uint256[] memory minAmountsOutScaled18,
address router,
RemoveLiquidityParams memory params,
PoolData memory poolData,
IHooks hooksContract
) internal {
if (
hooksContract.onBeforeRemoveLiquidity(
router,
params.pool,
params.kind,
params.maxBptAmountIn,
minAmountsOutScaled18,
poolData.balancesLiveScaled18,
params.userData
) == false
) {
revert IVaultErrors.BeforeRemoveLiquidityHookFailed();
}
}
function onBeforeRemoveLiquidity(
address,
address,
RemoveLiquidityKind,
uint256,
uint256[] memory,
uint256[] memory,
bytes memory
) public virtual returns (bool) {
return false;
}
But when adding liquidity it is overriding the default function
if (poolData.poolConfigBits.shouldCallAfterAddLiquidity()) {
IHooks hooksContract = _hooksContracts[params.pool];
amountsIn = poolData.poolConfigBits.callAfterAddLiquidityHook(
msg.sender,
amountsInScaled18,
amountsIn,
bptAmountOut,
params,
poolData,
hooksContract
);
}
function callAfterAddLiquidityHook(
PoolConfigBits config,
address router,
uint256[] memory amountsInScaled18,
uint256[] memory amountsInRaw,
uint256 bptAmountOut,
AddLiquidityParams memory params,
PoolData memory poolData,
IHooks hooksContract
) internal returns (uint256[] memory) {
(bool success, uint256[] memory hookAdjustedAmountsInRaw) = hooksContract.onAfterAddLiquidity(
router,
params.pool,
params.kind,
amountsInScaled18,
amountsInRaw,
bptAmountOut,
poolData.balancesLiveScaled18,
params.userData
);
}
function onAfterAddLiquidity(
address,
address pool,
AddLiquidityKind kind,
uint256[] memory,
uint256[] memory amountsInRaw,
uint256,
uint256[] memory,
bytes memory
) public override onlyVault returns (bool success, uint256[] memory) {
if (kind != AddLiquidityKind.PROPORTIONAL) {
return (false, amountsInRaw);
}
IERC20[] memory tokens = _vault.getPoolTokens(pool);
uint256[] memory hookAdjustedAmountsInRaw = amountsInRaw;
if (addLiquidityHookFeePercentage > 0) {
for (uint256 i = 0; i < amountsInRaw.length; i++) {
uint256 hookFee = amountsInRaw[i].mulUp(addLiquidityHookFeePercentage);
if (hookFee > 0) {
hookAdjustedAmountsInRaw[i] += hookFee;
_vault.sendTo(tokens[i], address(this), hookFee);
emit HookFeeCharged(address(this), tokens[i], hookFee);
}
}
}
return (true, hookAdjustedAmountsInRaw);
}
The UpliftOnlyExample contract inherits from BaseHooks but does not override the onBeforeRemoveLiquidity function, causing it to return false by default and making all liquidity removal operations revert.
Impact
DOS - Users are unable to remove liquidity from pools that have shouldCallBeforeRemoveLiquidity enabled, effectively locking their funds in the pool.
Tools Used
Manual Review
Recommendations
Override the onBeforeRemoveLiquidity function to return true so that operations can proceed. Additionally consider overide all relevant functions that would lead to a revert in execution and have them return true so that execution can proceed
