code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L119-L164 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/GrantFund.sol#L58-L68 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L366-L404

Vulnerability details

Impact

After the GrantFund contract, which inherits the StandardFunding contract, is deployed, treasury is initially 0. When someone calls the GrantFund.fundTreasury function below to increase treasury, a malicious actor can frontrun this GrantFund.fundTreasury transaction by calling the following StandardFunding.startNewDistributionPeriod function. Because _currentDistributionId and _distributions[currentDistributionId].endBlock are both 0 at that moment, calling the StandardFunding.startNewDistributionPeriod function would make block.number <= currentDistributionEndBlock false. Then, executing uint256 gbc = Maths.wmul(treasury, GLOBAL_BUDGET_CONSTRAINT) and newDistributionPeriod.fundsAvailable = SafeCast.toUint128(gbc) set both gbc and newDistributionPeriod.fundsAvailable, which is fundsAvailable for _distributions[1], to 0 since treasury is still 0 due to the frontrunning.

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

    function startNewDistributionPeriod() external override returns (uint24 newDistributionId_) {
        uint24  currentDistributionId       = _currentDistributionId;
        uint256 currentDistributionEndBlock = _distributions[currentDistributionId].endBlock;

        // check that there isn't currently an active distribution period
        if (block.number <= currentDistributionEndBlock) revert DistributionPeriodStillActive();

        ...

        // set the distribution period to start at the current block
        uint48 startBlock = SafeCast.toUint48(block.number);
        uint48 endBlock = startBlock + DISTRIBUTION_PERIOD_LENGTH;

        // set new value for currentDistributionId
        newDistributionId_ = _setNewDistributionId();

        // create QuarterlyDistribution struct
        QuarterlyDistribution storage newDistributionPeriod = _distributions[newDistributionId_];
        newDistributionPeriod.id              = newDistributionId_;
        newDistributionPeriod.startBlock      = startBlock;
        newDistributionPeriod.endBlock        = endBlock;
        uint256 gbc                           = Maths.wmul(treasury, GLOBAL_BUDGET_CONSTRAINT);
        newDistributionPeriod.fundsAvailable  = SafeCast.toUint128(gbc);

        // decrease the treasury by the amount that is held for allocation in the new distribution period
        treasury -= gbc;

        ...
    }

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/GrantFund.sol#L58-L68

    function fundTreasury(uint256 fundingAmount_) external override {
        IERC20 token = IERC20(ajnaTokenAddress);

        // update treasury accounting
        treasury += fundingAmount_;

        ...
    }

After the frontrunning, although treasury becomes positive after the GrantFund.fundTreasury transaction is executed, fundsAvailable for _distributions[1] remains 0. For _distributions[_currentDistributionId], which is _distributions[1], calling the following StandardFunding.proposeStandard function for any proposal with a positive tokensRequested will revert because newProposal.tokensRequested > (currentDistribution.fundsAvailable * 9 / 10), which is evaluated to newProposal.tokensRequested > 0, would be true. Because it is meaningless to create proposals for requesting 0 AJNA tokens, no one would use the StandardFunding contract for _distributions[1]. As a result, the StandardFunding contract's functionalities for proposals with positive tokensRequested are DOS'ed for _distributions[1], which lasts at least 648000 blocks or about 90 days that is not a short time.

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L366-L404

    function proposeStandard(
        address[] memory targets_,
        uint256[] memory values_,
        bytes[] memory calldatas_,
        string memory description_
    ) external override returns (uint256 proposalId_) {
        proposalId_ = _hashProposal(targets_, values_, calldatas_, keccak256(abi.encode(DESCRIPTION_PREFIX_HASH_STANDARD, keccak256(bytes(description_)))));

        Proposal storage newProposal = _standardFundingProposals[proposalId_];

        // check for duplicate proposals
        if (newProposal.proposalId != 0) revert ProposalAlreadyExists();

        QuarterlyDistribution memory currentDistribution = _distributions[_currentDistributionId];

        // cannot add new proposal after end of screening period
        // screening period ends 72000 blocks before end of distribution period, ~ 80 days.
        if (block.number > _getScreeningStageEndBlock(currentDistribution.endBlock)) revert ScreeningPeriodEnded();

        // store new proposal information
        newProposal.proposalId      = proposalId_;
        newProposal.distributionId  = currentDistribution.id;
        newProposal.tokensRequested = _validateCallDatas(targets_, values_, calldatas_); // check proposal parameters are valid and update tokensRequested

        // revert if proposal requested more tokens than are available in the distribution period
        if (newProposal.tokensRequested > (currentDistribution.fundsAvailable * 9 / 10)) revert InvalidProposal();

        ...
    }

Proof of Concept

The following steps can occur for the described scenario.

After the GrantFund contract is deployed, someone calls the GrantFund.fundTreasury function.
A malicious actor frontruns the GrantFund.fundTreasury transaction by calling the StandardFunding.startNewDistributionPeriod function.
After the frontrunning, fundsAvailable for _distributions[1] remains 0.
For _distributions[_currentDistributionId], which is _distributions[1], calling the StandardFunding.proposeStandard function for any proposal with a positive tokensRequested will revert.
Since no user can create a proposal with a positive tokensRequested for _distributions[1] using the StandardFunding contract, this contract's functionalities for such proposals are DOS'ed for _distributions[1].