Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crazy Cyan Worm - Residual Allowance in DEX Handler Enables PAIRED_LP_TOKEN Drain #562

Open
sherlock-admin3 opened this issue Feb 17, 2025 · 0 comments

Comments

@sherlock-admin3
Copy link
Contributor

Crazy Cyan Worm

Medium

Residual Allowance in DEX Handler Enables PAIRED_LP_TOKEN Drain

Summary

The missing allowance reset in DecentralizedIndex.addLiquidityV2 will cause partial loss of PAIRED_LP_TOKEN reserves for the protocol as attackers will exploit residual allowances through DEX operations. The root cause in DecentralizedIndex.sol#L358 is the use of safeIncreaseAllowance(0) instead of forceApprove(0) when resetting DEX handler permissions after the external calling DEX_HANDLER.addLiquidity, leaving active allowances that can be abused through standard ERC20 transfers.

Root Cause

In function DecentralizedIndex.addLiquidityV2(DecentralizedIndex.sol#L358) the incorrect allowance reset mechanism uses safeIncreaseAllowance instead of forceApprove to clear remaining token allowances. This leaves residual allowance for the DEX handler contract after liquidity operations, violating the principle of least privilege for ERC20 token approvals. The code should explicitly reset allowances to zero after external calls to prevent potential misuse of leftover permissions:

    function addLiquidityV2(
        uint256 _pTKNLPTokens,
        uint256 _pairedLPTokens,
        uint256 _slippage, // 100 == 10%, 1000 == 100%
        uint256 _deadline
    ) external override lock noSwapOrFee returns (uint256) {
				// ...

        IERC20(PAIRED_LP_TOKEN).safeTransferFrom(_msgSender(), address(this), _pairedLPTokens);
        IERC20(PAIRED_LP_TOKEN).safeIncreaseAllowance(address(DEX_HANDLER), _pairedLPTokens);

        uint256 _poolBalBefore = IERC20(DEX_HANDLER.getV2Pool(address(this), PAIRED_LP_TOKEN)).balanceOf(_msgSender());
        DEX_HANDLER.addLiquidity(
            address(this),
            PAIRED_LP_TOKEN,
            _pTKNLPTokens,
            _pairedLPTokens,
            (_pTKNLPTokens * (1000 - _slippage)) / 1000,
            (_pairedLPTokens * (1000 - _slippage)) / 1000,
            _msgSender(),
            _deadline
        );
@>      IERC20(PAIRED_LP_TOKEN).safeIncreaseAllowance(address(DEX_HANDLER), 0);
        
        // ...
     }

Internal Pre-conditions

  1. DecentralizedIndex holds PAIRED_LP_TOKEN balance (e.g. 500 tokens worth $5,000)
  2. Residual allowance remains after liquidity operations (e.g. 50 tokens left from 500 token approval)
  3. Protocol accumulates multiple liquidity positions over time (e.g. 10 transactions leaving 50 tokens allowance each = 500 tokens total exposure)

External Pre-conditions

  1. DEX_HANDLER implements standard ERC20 transferFrom functionality using existing allowances
  2. PAIRED_LP_TOKEN has liquid market (e.g. $10/token on secondary markets)
  3. Protocol uses upgradable DEX_HANDLER contract or multiple approved handlers

Attack Path

  1. Legitimate user adds liquidity with 500 PAIRED_LP_TOKEN approval (actual usage 450 tokens)
  2. DecentralizedIndex leaves 50 token allowance for DEX_HANDLER
  3. Attacker calls DEX_HANDLER's swap function with malicious parameters:
    • Uses transferFrom to take 50 PAIRED_LP_TOKEN from DecentralizedIndex
    • Swaps stolen tokens for ETH through normal DEX operations
  4. Attacker repeats for each residual allowance instance across all historical transactions
  5. Protocol loses 10% of each liquidity addition's paired tokens (500 tokens total = $5,000 loss)

Impact

Attackers can systematically drain 10-20% of every liquidity addition's paired tokens through normal DEX operations. For a protocol with $100,000 in total liquidity additions, this results in $10,000-$20,000 direct loss. The impact scales linearly with protocol usage, creating a perpetual leakage of paired assets.

PoC

No response

Mitigation

Replace safeIncreaseAllowance with forceApprove to fully reset the allowance after liquidity operations. This ensures the DEX handler contract cannot use residual approvals in future transactions. The OpenZeppelin forceApprove function atomically sets allowance to zero before updating, preventing any leftover permissions from remaining active.

-       IERC20(PAIRED_LP_TOKEN).safeIncreaseAllowance(address(DEX_HANDLER), 0);
+       IERC20(PAIRED_LP_TOKEN).forceApprove(address(DEX_HANDLER), 0);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant