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

Swap fees are frozen forever in UpliftOnlyExample contract because there is no way to withdraw

perseverance

Summary

In the case that hookSwapFeePercentage of UpliftOnlyExample contract is configured bigger than 0. This is likely to happen because the contract is intended to charge some fees for swap service. In the test case of the contract, the minimum fee is 0.001%.

The swap fee is sent to this UpliftOnlyExample contract from the Vault contract. Swap fees are frozen forever in UpliftOnlyExample contract because there is no way to withdraw the fee token.

Vulnerability Details

The root cause is that there is no function to withdraw the fee. So in the case, the hook fee is > 0 the Vault will send the token to the UpliftOnlyExample contract. So the fee token will be frozen forever.

https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L293-L298

https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L342-L350

function onAfterSwap(
AfterSwapParams calldata params
) public override onlyVault returns (bool success, uint256 hookAdjustedAmountCalculatedRaw) {
hookAdjustedAmountCalculatedRaw = params.amountCalculatedRaw;
if (hookSwapFeePercentage > 0) {
uint256 hookFee = params.amountCalculatedRaw.mulUp(hookSwapFeePercentage);
​
if (hookFee > 0) {
// Removed for simplicity
if (ownerFee > 0) {
_vault.sendTo(feeToken, address(this), ownerFee);
​
emit SwapHookFeeCharged(address(this), feeToken, ownerFee);
}
}
}
return (true, hookAdjustedAmountCalculatedRaw);
}

Impact

Impact: High

Reason: Because the swap fee that is money are stuck (frozen) forever in the contract. The amount of money lost is accumulated over time and eventually become quite big.

Likelyhood: High

It's highly to happen. The hookSwapFeePercentage is likely to be configured as > 0. In the test case, there is configuration of min and max swap fee. See testFeeSwapExactIn__Fuzz for example.

https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/test/foundry/UpliftExample.t.sol#L56-L57

// Maximum exit fee of 10%
uint64 private constant _MIN_SWAP_FEE_PERCENTAGE = 0.001e16; // 0.001%
uint64 private constant _MAX_SWAP_FEE_PERCENTAGE = 10e16; // 10%

=> Total Impact: High

Reference: https://docs.codehawks.com/hawks-auditors/how-to-evaluate-a-finding-severity

Tools Used

Foundry test case

Proof of Concept

Step 1: Setup the precondition that hookSwapFeePercentage is > 0 for example, 5e16.

Step 2: Get the USDC balance of the contract upliftOnlyRouter that is 0. An user (Bob) swaps DAI for USDC.
Check the USDC balance of the upliftOnlyRouter now is > 0

POC code below:

Copy the test testFeeSwapExactIn into https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/test/foundry/UpliftExample.t.sol

function testFeeSwapExactIn() public {
uint256 swapAmount = 1000*10**18;
​
console.log("Setup the scenario the swapFee is 5e16");
// Fee between 0 and 100%
uint64 hookFeePercentage = 5e16;
​
vm.expectEmit();
emit UpliftOnlyExample.HookSwapFeePercentageChanged(address(upliftOnlyRouter), hookFeePercentage);
​
vm.prank(owner);
upliftOnlyRouter.setHookSwapFeePercentage(hookFeePercentage);
uint256 hookFee = swapAmount.mulUp(hookFeePercentage);
console.log("Before swap: USDC balance of uplift Router ", IERC20(usdc).balanceOf(address(upliftOnlyRouter)));
​
vm.prank(bob);
vm.expectCall(
address(upliftOnlyRouter),
abi.encodeCall(
IHooks.onAfterSwap,
AfterSwapParams({
kind: SwapKind.EXACT_IN,
tokenIn: dai,
tokenOut: usdc,
amountInScaled18: swapAmount,
amountOutScaled18: swapAmount,
tokenInBalanceScaled18: poolInitAmount + swapAmount,
tokenOutBalanceScaled18: poolInitAmount - swapAmount,
amountCalculatedScaled18: swapAmount,
amountCalculatedRaw: swapAmount,
router: address(router),
pool: pool,
userData: bytes("")
})
)
);
​
if (hookFee > 0) {
vm.expectEmit();
emit UpliftOnlyExample.SwapHookFeeCharged(address(upliftOnlyRouter), IERC20(usdc), hookFee);
}
console.log("User Bob swap dai for usdc");
router.swapSingleTokenExactIn(address(pool), dai, usdc, swapAmount, 0, MAX_UINT256, false, bytes(""));
​
console.log("After swap: USDC balance of uplift Router ", IERC20(usdc).balanceOf(address(upliftOnlyRouter)));
}

Run test:

forge test --match-test testFeeSwapExactIn -vv

Test log:

[PASS] testFeeSwapExactIn() (gas: 216871)
Logs:
Setup the scenario the swapFee is 5e16
Before swap: USDC balance of uplift Router 0
User Bob swap dai for usdc
After swap: USDC balance of uplift Router 50000000000000000000
​

Recommendations

Implement some function to withdraw the fees. Or use some admin EOA to receive the fee.

@note In the hook examples, FeeTakingHookExample implements withdrawFees functionality. But the upliftExampleOnly now does not inherit or have any similar function. In the test case of 2024-12-quantamm\pkg\pool-hooks\test\foundry\UpliftExample.t.sol, I see some test related to FeeTakingHookExample, but it is not applicable for upliftExampleOnly.

Updates

Lead Judging Commences

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

finding_ownerFee_cannot_be_withdrawn

Likelihood: High, every swap. Impact: High, funds are stuck.

Support

FAQs

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

Give us feedback!