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

freeze of funds (fees) to `QuantAMMAdmin` in `onAfterRemoveLiquidity()` fees distribution

0xhuntoor

Summary

in UpliftOnlyExample::onAfterRemoveLiquidity() the function sends the QuantAMMAdmin to the admin in the wrong way, making it non retrievable for the admin

Vulnerability Details

in UpliftOnlyExample::onAfterRemoveLiquidity(), the fees are sent the to QuantAMMAdmin as a BPT tokens here

File: UpliftOnlyExample.sol
431: function onAfterRemoveLiquidity(
​
536: if (localData.adminFeePercent > 0) {
537: _vault.addLiquidity(
538: AddLiquidityParams({
539: pool: localData.pool,
540: to: IUpdateWeightRunner(_updateWeightRunner).getQuantAMMAdmin(),
541: maxAmountsIn: localData.accruedQuantAMMFees,
542: minBptAmountOut: localData.feeAmount.mulDown(localData.adminFeePercent) / 1e18,
543: kind: AddLiquidityKind.PROPORTIONAL,
544: userData: bytes("")
​

But the problem is that when the admin wants to exchange those BPT tokens, he can't do it through UpliftOnlyExample directly, since the admin doesn't have LPNFT token or a stored position in poolsFeeData failing his txn in onAfterRemoveLiquidity by this loop that will underflow if he doesn't have enough BPT token balance registered in the contract here

471: for (uint256 i = localData.feeDataArrayLength - 1; i >= 0; --i) {

this will underflow cause his desired withdrawal balance haven't been substracted during the loop to break it here

504: if (localData.amountLeft == 0) {
505: break;
506: }
507: } else {
508: feeDataArray[i].amount -= localData.amountLeft;
509: localData.feeAmount += (feePerLP * localData.amountLeft);
510: break;
511: }

And if he tries to remove directly from BalancerV3 vault, he can't since balancer calls onAfterRemoveLiquidity with msg.sender as the router, which will fail due to onlySelfRouter modifier

File: Vault.sol
846: // Uses msg.sender as the Router (the contract that called the Vault).
847: if (poolData.poolConfigBits.shouldCallAfterRemoveLiquidity()) {
848: // `hooksContract` needed to fix stack too deep.
849: IHooks hooksContract = _hooksContracts[params.pool];
850:
851: amountsOut = poolData.poolConfigBits.callAfterRemoveLiquidityHook(
852: msg.sender,
853: amountsOutScaled18,
854: amountsOut,
855: bptAmountIn,
856: params,
857: poolData,
858: hooksContract
859: );
860: }
​
................
​
File: UpliftOnlyExample.sol
431: function onAfterRemoveLiquidity(
432: address router,
433: address pool,
434: RemoveLiquidityKind,
435: uint256 bptAmountIn,
436: uint256[] memory,
437: uint256[] memory amountsOutRaw,
438: uint256[] memory,
439: bytes memory userData
440: ) public override onlySelfRouter(router) returns (bool, uint256[] memory hookAdjustedAmountsOutRaw) {
​
.................
​
190: modifier onlySelfRouter(address router) {
191: _ensureSelfRouter(router);
192: _;
193: }
..................
​
683: function _ensureSelfRouter(address router) private view {
684: if (router != address(this)) {
685: revert CannotUseExternalRouter(router);
686: }
687: }
​

This way, the admin is holding useless BPT tokens that can't be exchanged to tokens

Impact

Loss of funds (fees) to QuantAMMAdmin

Tools Used

Manual Review

Recommendations

  • Implement position registering to admin and mint NFT to him (complex and gas costly)

  • Implement a logic to send Tokens to admin and not adding liquidity to him (easier and less gas costly)

Updates

Lead Judging Commences

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

finding_Uplift_admin_cannot_withdraw_without_nft

Likelihood: High, won’t be able to withdraw. Impact: High, funds stuck.

Support

FAQs

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