code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L85-L105

Vulnerability details

Since standard and extraordinary proposals use the same treasury funds accounting variables and extraordinary voting period is long enough (1 month), it is possible that extraordinary proposal that was valid and gained enough votes will end up frozen: it might not being able to be executed as the total treasury funds are decreased by standard proposals workflow and success criteria for the extraordinary one will not be satisfied by a small enough margin during its execution.

Impact

Otherwise valid extraordinary proposals will not be possible to be executed and repetition of the lengthy voting process will be needed.

That can be crucial as some proposals might be time-sensitive (as an example, some rescue funding).

Proof of Concept

It is checked that uint256(totalTokensRequested) > _getSliceOfTreasury(Maths.WAD - _getMinimumThresholdPercentage()) = Maths.wmul(treasury, Maths.WAD - _getMinimumThresholdPercentage()) on proposal creation:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L85-L105

    function proposeExtraordinary(
        uint256 endBlock_,
        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_EXTRAORDINARY, keccak256(bytes(description_)))));

        ExtraordinaryFundingProposal storage newProposal = _extraordinaryFundingProposals[proposalId_];

        // check if proposal already exists (proposal id not 0)
        if (newProposal.proposalId != 0) revert ProposalAlreadyExists();

        // check proposal length is within limits of 1 month maximum
        if (block.number + MAX_EFM_PROPOSAL_LENGTH < endBlock_) revert InvalidProposal();

        uint128 totalTokensRequested = _validateCallDatas(targets_, values_, calldatas_);

        // check tokens requested are available for claiming from the treasury
>>      if (uint256(totalTokensRequested) > _getSliceOfTreasury(Maths.WAD - _getMinimumThresholdPercentage())) revert InvalidProposal();

Extraordinary proposals can take a while, up to 1 month:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L20-L23

    /**
     * @notice The maximum length of a proposal's voting period, in blocks.
     */
    uint256 internal constant MAX_EFM_PROPOSAL_LENGTH = 216_000; // number of blocks in one month

_extraordinaryProposalSucceeded(proposalId_, tokensRequested) is required on proposal execution:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L56-L70

    function executeExtraordinary(
        address[] memory targets_,
        uint256[] memory values_,
        bytes[] memory calldatas_,
        bytes32 descriptionHash_
    ) external nonReentrant override returns (uint256 proposalId_) {
        proposalId_ = _hashProposal(targets_, values_, calldatas_, keccak256(abi.encode(DESCRIPTION_PREFIX_HASH_EXTRAORDINARY, descriptionHash_)));

        ExtraordinaryFundingProposal storage proposal = _extraordinaryFundingProposals[proposalId_];

        // since we are casting from uint128 to uint256, we can safely assume that the value will not overflow
        uint256 tokensRequested = uint256(proposal.tokensRequested);

        // check proposal is succesful and hasn't already been executed
        if (proposal.executed || !_extraordinaryProposalSucceeded(proposalId_, tokensRequested)) revert ExecuteExtraordinaryProposalInvalid();

getExtraordinaryProposalSucceeded() checks for `votesReceived >= tokensRequested + getSliceOfNonTreasury(minThresholdPercentageandtokensRequested <= _getSliceOfTreasury(Maths.WAD - minThresholdPercentage)`:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/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))
        ;
    }

While _getMinimumThresholdPercentage() is stable within given extraordinary proposal:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L206-L215

    function _getMinimumThresholdPercentage() internal view returns (uint256) {
        // default minimum threshold is 50
        if (_fundedExtraordinaryProposals.length == 0) {
            return 0.5 * 1e18;
        }
        // minimum threshold increases according to the number of funded EFM proposals
        else {
            return 0.5 * 1e18 + (_fundedExtraordinaryProposals.length * (0.05 * 1e18));
        }
    }

treasury is reduced by 3% on each new standard distribution period start, which can be in the middle of the extraordinary proposal voting period:

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

    function startNewDistributionPeriod() external override returns (uint24 newDistributionId_) {
        ...

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

        // create QuarterlyDistribution struct
        ...
        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/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/StandardFunding.sol#L23-L27

    /**
     * @notice Maximum percentage of tokens that can be distributed by the treasury in a quarter.
     * @dev Stored as a Wad percentage.
     */
    uint256 internal constant GLOBAL_BUDGET_CONSTRAINT = 0.03 * 1e18;

Both votesReceived >= tokensRequested_ + _getSliceOfNonTreasury(minThresholdPercentage and tokensRequested_ <= _getSliceOfTreasury(Maths.WAD - minThresholdPercentage) conditions are worse off when treasury is reduced:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L222-L227

    function _getSliceOfNonTreasury(
        uint256 percentage_
    ) internal view returns (uint256) {
        uint256 totalAjnaSupply = IERC20(ajnaTokenAddress).totalSupply();
        return Maths.wmul(totalAjnaSupply - treasury, percentage_);
    }

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L234-L238

    function _getSliceOfTreasury(
        uint256 percentage_
    ) internal view returns (uint256) {
        return Maths.wmul(treasury, percentage_);
    }

I.e. a proposal that is valid and gained enough votes can be frozen as one of these conditions can become unavailable due to routine decrease of treasury.

Recommended Mitigation Steps

Since standard workflow decrease can happen only once per extraordinary proposal voting, consider adding a cushion for such possibility, for example (GLOBAL_BUDGET_CONSTRAINT has to be moved to the parent code for visibility):

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L85-L105

    function proposeExtraordinary(
        uint256 endBlock_,
        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_EXTRAORDINARY, keccak256(bytes(description_)))));

        ExtraordinaryFundingProposal storage newProposal = _extraordinaryFundingProposals[proposalId_];

        // check if proposal already exists (proposal id not 0)
        if (newProposal.proposalId != 0) revert ProposalAlreadyExists();

        // check proposal length is within limits of 1 month maximum
        if (block.number + MAX_EFM_PROPOSAL_LENGTH < endBlock_) revert InvalidProposal();

        uint128 totalTokensRequested = _validateCallDatas(targets_, values_, calldatas_);

        // check tokens requested are available for claiming from the treasury
-       if (uint256(totalTokensRequested) > _getSliceOfTreasury(Maths.WAD - _getMinimumThresholdPercentage())) revert InvalidProposal();
+       if (uint256(totalTokensRequested) > (Maths.WAD - GLOBAL_BUDGET_CONSTRAINT) * _getSliceOfTreasury(Maths.WAD - _getMinimumThresholdPercentage()) / Maths.WAD) revert InvalidProposal();