Ajna Protocol - rvierdiiev's results

Lines of code

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

Vulnerability details

Impact

PositionManager.memorializePositions can be called on behalf of owner. Anyone can link indexes of owner to specific owner's token.

Proof of Concept

PositionManager contract is created to allow user to link their indexes to specific token that can be then sold. So users will be using PositionManager to mint several tokens and link some indexes to one token and some indexes to another token. In this way they will have tokens portfolio with related indexes.

When user created tokens and he wants to link indexes, then first he needs to approve those indexes to PositionManager contract. After that user can call PositionManager.memorializePositions and provide positions that he wants to add to the provided token.

The problem is that this function is permissionless. Anyone can call it and provide token id and indexes. So anyone can call it on behalf of token's and indexes owner.

In this way caller will make problems to owner, as he can link indexes to wrong token(owner wanted to link them to token1, but attacker linked to token2) or he can link indexes that owner was not going to add anymore(but already provided approve).

Tools Used

VsCode

Recommended Mitigation Steps

Allow only owner or approved account to call this function.

Assessed type

Access Control

Lines of code

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

Vulnerability details

Impact

User can avoid bankrupting by calling PositionManager.moveLiquidity where to index is bankrupted index

Proof of Concept

Bucket could become insolvent and in that case all LP within the bucket are zeroed out (lenders lose all their LP). Because of that, PositionManager.reedemPositions will not allow to redeem index that is bankrupted.

When user wants to move his LPs from one bucket to another he can call PositionManager.moveLiquidity where he will provide from and to indexes. https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L262-L333

    function moveLiquidity(
        MoveLiquidityParams calldata params_
    ) external override mayInteract(params_.pool, params_.tokenId) nonReentrant {
        Position storage fromPosition = positions[params_.tokenId][params_.fromIndex];

        MoveLiquidityLocalVars memory vars;
        vars.depositTime = fromPosition.depositTime;

        // handle the case where owner attempts to move liquidity after they've already done so
        if (vars.depositTime == 0) revert RemovePositionFailed();

        // ensure bucketDeposit accounts for accrued interest
        IPool(params_.pool).updateInterest();

        // retrieve info of bucket from which liquidity is moved  
        (
            vars.bucketLP,
            vars.bucketCollateral,
            vars.bankruptcyTime,
            vars.bucketDeposit,
        ) = IPool(params_.pool).bucketInfo(params_.fromIndex);

        // check that bucket hasn't gone bankrupt since memorialization
        if (vars.depositTime <= vars.bankruptcyTime) revert BucketBankrupt();

        // calculate the max amount of quote tokens that can be moved, given the tracked LP
        vars.maxQuote = _lpToQuoteToken(
            vars.bucketLP,
            vars.bucketCollateral,
            vars.bucketDeposit,
            fromPosition.lps,
            vars.bucketDeposit,
            _priceAt(params_.fromIndex)
        );

        EnumerableSet.UintSet storage positionIndex = positionIndexes[params_.tokenId];

        // remove bucket index from which liquidity is moved from tracked positions
        if (!positionIndex.remove(params_.fromIndex)) revert RemovePositionFailed();

        // update bucket set at which a position has liquidity
        // slither-disable-next-line unused-return
        positionIndex.add(params_.toIndex);

        // move quote tokens in pool
        (
            vars.lpbAmountFrom,
            vars.lpbAmountTo,
        ) = IPool(params_.pool).moveQuoteToken(
            vars.maxQuote,
            params_.fromIndex,
            params_.toIndex,
            params_.expiry
        );

        Position storage toPosition = positions[params_.tokenId][params_.toIndex];

        // update position LP state
        fromPosition.lps -= vars.lpbAmountFrom;
        toPosition.lps   += vars.lpbAmountTo;
        // update position deposit time to the from bucket deposit time
        toPosition.depositTime = vars.depositTime;

        emit MoveLiquidity(
            ownerOf(params_.tokenId),
            params_.tokenId,
            params_.fromIndex,
            params_.toIndex,
            vars.lpbAmountFrom,
            vars.lpbAmountTo
        );
    }

As you can see from bucket is checked to be not bankrupted before the moving. And after the move, LPs of from and to buckets are updated. Also depositTime of to bucket is updated to from.depositTime.

The problem here is that to bucket was never checked to be not bankrupted. Because of that it's possible that bankrupted to bucket now becomes not bankrupted as their depositTime is updated now.

This is how this can be used by attacker. 1.Attacker has lp shares in the bucket, linked to token and this bucket became bankrupt. 2.Then attacker mints small amount of LP in the Pool and then memorizes this index to the token. 3.Attacker calls moveLiquidity with from: new bucket and to: bankrupt bucket. 4.Now attacker can redeem his lp shares from bankrupt bucket as depositedTime is updated now.

As result, attacker was able to steal LPs of another people from PositionManager contract.

Tools Used

VsCode

Recommended Mitigation Steps

In case if to bucket is bankrupt, then clear LP for it before adding moved lp shares.

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/Funding.sol#L76-L93

Vulnerability details

Impact

Because Funding._getVotesAtSnapshotBlocks function substitutes voteStartBlock_ with block.number - 1 when voteStartBlock_ == block.number attacker can vote without votes.

Proof of Concept

Funding._getVotesAtSnapshotBlocks function calculates amount of votes, that user has. https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/Funding.sol#L76-L93

    function _getVotesAtSnapshotBlocks(
        address account_,
        uint256 snapshot_,
        uint256 voteStartBlock_
    ) internal view returns (uint256) {
        IVotes token = IVotes(ajnaTokenAddress);

        // calculate the number of votes available at the snapshot block
        uint256 votes1 = token.getPastVotes(account_, snapshot_);

        // enable voting weight to be calculated during the voting period's start block
        voteStartBlock_ = voteStartBlock_ != block.number ? voteStartBlock_ : block.number - 1;

        // calculate the number of votes available at the stage's start block
        uint256 votes2 = token.getPastVotes(account_, voteStartBlock_);

        return Maths.min(votes2, votes1);
    }

It takes votes amount at 2 time points: snapshot_ and voteStartBlock_. Then minimum amount is user's votes amount. In case if voteStartBlock_ == block.number, voteStartBlock_ will be set to block.number - 1.

Funding._getVotesAtSnapshotBlocks function is used in several places in the code. We will look into ExtraordinaryFunding.voteExtraordinary function.

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L131-L157

        function voteExtraordinary(
        uint256 proposalId_
    ) external override returns (uint256 votesCast_) {
        // revert if msg.sender already voted on proposal
        if (hasVotedExtraordinary[proposalId_][msg.sender]) revert AlreadyVoted();

        ExtraordinaryFundingProposal storage proposal = _extraordinaryFundingProposals[proposalId_];
        // revert if proposal is inactive
        if (proposal.startBlock > block.number || proposal.endBlock < block.number || proposal.executed) {
            revert ExtraordinaryFundingProposalInactive();
        }

        // check voting power at snapshot block and update proposal votes
        votesCast_ = _getVotesExtraordinary(msg.sender, proposalId_);
        proposal.votesReceived += SafeCast.toUint120(votesCast_);

        // record that voter has voted on this extraordinary funding proposal
        hasVotedExtraordinary[proposalId_][msg.sender] = true;

        emit VoteCast(
            msg.sender,
            proposalId_,
            1,
            votesCast_,
            ""
        );
    }

First of all, user is allowed to call voteExtraordinary function right on the proposal.startBlock, just when proposal was created. Then _getVotesExtraordinary function is called to calculate user's votes for that proposal. https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L246-L256

    function _getVotesExtraordinary(address account_, uint256 proposalId_) internal view returns (uint256 votes_) {
        if (proposalId_ == 0) revert ExtraordinaryFundingProposalInactive();

        uint256 startBlock = _extraordinaryFundingProposals[proposalId_].startBlock;

        votes_ = _getVotesAtSnapshotBlocks(
            account_,
            startBlock - VOTING_POWER_SNAPSHOT_DELAY,
            startBlock
        );
    }

As you can see this function will call _getVotesAtSnapshotBlocks and pass startBlock param. In case if voteExtraordinary was called just on startBlock of proposal, then startBlock == block.number and it will be decreased by 1.

That means that voter can actually don't have any votes in startBlock, but he will still be able to use them.

Example: 1.User has 100_000 ajna tokens from block 0 till block 50. 2.On block 50 user sold all his tokens. 3.On block 51 new proposal is created. 4.At same block attacker votes with all his 100_000 tokens, as votes were taken not from 51th, but 50th block. However now he don't have any ajna tokens.

This allows anyone who has big amount of tokens to create proposal and vote on it right after selling their tokens.

Tools Used

VsCode

Recommended Mitigation Steps

Do not decrease voteStartBlock_.

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L70 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L164-L178

Vulnerability details

Impact

ExtraordinaryFunding doesn't work as described in whitepaper.

Proof of Concept

ExtraordinaryFunding is needed to create proposal that can transfer more than 3% of treasury as grant. This is how it's checked if proposal succeeded. https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L164-L178

    function _extraordinaryProposalSucceeded(
        uint256 proposalId_,
        uint256 tokensRequested_
    ) internal view returns (bool) {
        uint256 votesReceived          = uint256(_extraordinaryFundingProposals[proposalId_].votesReceived);
        uint256 minThresholdPercentage = _getMinimumThresholdPercentage();

        return
            // succeeded if proposal's votes received doesn't exceed the minimum threshold required
            (votesReceived >= tokensRequested_ + _getSliceOfNonTreasury(minThresholdPercentage))
            &&
            // succeeded if tokens requested are available for claiming from the treasury
            (tokensRequested_ <= _getSliceOfTreasury(Maths.WAD - minThresholdPercentage))
        ;
    }

Here, minThresholdPercentage is the percentage of non treasury votes. It's 50% for the first proposal and increases by 5% for each new proposal.

Inside 9.2.2 Extraordinary Funding Mechanism (EFM) of whitepaper there is description of how this should work. We need to check point 4 here.

4. When submitting a proposal the proposer must include the exact number of tokens they
would like to extract, proposal threshold. This proposal threshold is added to the
minimum threshold to provide a required votes received threshold. If the vote fails to
reach this threshold it will fail and no tokens will be distributed. Example:
a. A proposer requests tokens equivalent to 10% of the treasury
i. 50% + 10% = 60%
ii. If 65% of non-treasury tokens vote affirmatively, 10% of the treasury is
released
iii. If 59.9% of non-treasury tokens vote affirmatively, 0% of the treasury is
released

This means that when proposer creates new proposal then he should provide amount that he wants this proposal to receive(tokensRequested). By dividing treasury amount by proposal wanted amount we will receive treasury percentage that proposal wants(treasury/tokensRequested=proposalThreshold). These treasury percentage is additional threshold according to the whitepaper. And whitepaper says that in order to suceed proposal should have minThresholdPercentage + proposalThreshold of NON treasury funds.

This is not like that in the code and this condition is checked like that: votesReceived >= tokensRequested_ + _getSliceOfNonTreasury(minThresholdPercentage) The problem here is that tokensRequested_ is proposalThreshold of treasury, not NON treasury. And because of that tokensRequested_ + _getSliceOfNonTreasury(minThresholdPercentage) will not be minThresholdPercentage + proposalThreshold percentage of NON treasury funds

Let's check simple example. 1.non treasury is 1 million, and _getMinimumThresholdPercentage is 50%, so it's 500_000 votes 2.treasury is 100_000 3.user creates proposal to withdraw 50_000 tokens from treasury(50% of treasury). This means that all 100% of non treasury should vote according to whitepaper. 4.however it's enough 550_000 votes to pass proposal

Tools Used

VsCode

Recommended Mitigation Steps

When create proposal you can store proposalThreshold as percentage of treasury funds that were asked. Then check votes like this: votesReceived >= _getSliceOfNonTreasury(minThresholdPercentage + proposalThreshold)

Assessed type

Error