QuantAMM

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

UpliftOnlyExample.sol :: onAfterSwap() will remain permanently locked in the contract as there is no method implemented to withdraw the ERC20 tokens.

Summary

onAfterSwap() is a hook used to calculate the fees that users must pay to QuantAMM and the owner. The owner's fee is deposited into the contract. However, the contract lacks a function to withdraw the ERC20 tokens, resulting in the funds being permanently locked within the contract.

Vulnerability Details

onAfterSwap() is implemented as follows.

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) {
IERC20 feeToken;
// Note that we can only alter the calculated amount in this function. This means that the fee will be
// charged in different tokens depending on whether the swap is exact in / out, potentially breaking
// the equivalence (i.e., one direction might "cost" less than the other).
if (params.kind == SwapKind.EXACT_IN) {
// For EXACT_IN swaps, the `amountCalculated` is the amount of `tokenOut`. The fee must be taken
// from `amountCalculated`, so we decrease the amount of tokens the Vault will send to the caller.
//
// The preceding swap operation has already credited the original `amountCalculated`. Since we're
// returning `amountCalculated - hookFee` here, it will only register debt for that reduced amount
// on settlement. This call to `sendTo` pulls `hookFee` tokens of `tokenOut` from the Vault to this
// contract, and registers the additional debt, so that the total debits match the credits and
// settlement succeeds.
feeToken = params.tokenOut;
hookAdjustedAmountCalculatedRaw -= hookFee;
} else {
// For EXACT_OUT swaps, the `amountCalculated` is the amount of `tokenIn`. The fee must be taken
// from `amountCalculated`, so we increase the amount of tokens the Vault will ask from the user.
//
// The preceding swap operation has already registered debt for the original `amountCalculated`.
// Since we're returning `amountCalculated + hookFee` here, it will supply credit for that increased
// amount on settlement. This call to `sendTo` pulls `hookFee` tokens of `tokenIn` from the Vault to
// this contract, and registers the additional debt, so that the total debits match the credits and
// settlement succeeds.
feeToken = params.tokenIn;
hookAdjustedAmountCalculatedRaw += hookFee;
}
uint256 quantAMMFeeTake = IUpdateWeightRunner(_updateWeightRunner).getQuantAMMUpliftFeeTake();
uint256 ownerFee = hookFee;
if (quantAMMFeeTake > 0) {
uint256 adminFee = hookFee / (1e18 / quantAMMFeeTake);
ownerFee = hookFee - adminFee;
address quantAMMAdmin = IUpdateWeightRunner(_updateWeightRunner).getQuantAMMAdmin();
_vault.sendTo(feeToken, quantAMMAdmin, adminFee);
emit SwapHookFeeCharged(quantAMMAdmin, feeToken, adminFee);
}
if (ownerFee > 0) {
@> _vault.sendTo(feeToken, address(this), ownerFee);
emit SwapHookFeeCharged(address(this), feeToken, ownerFee);
}
}
}
return (true, hookAdjustedAmountCalculatedRaw);
}

As observed, if ownerFee > 0, the tokens are deposited into the contract. The issue lies in the fact that the contract lacks any function to withdraw these ERC20 tokens, causing the funds to remain permanently locked within the contract.

Impact

Loss of funds occurs as the ERC20 tokens will remain permanently locked in the contract.

Tools Used

Manual review.

Recommendations

Add a function to enable the withdrawal of collected fees (ERC20).

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!