code-423n4 / 2023-05-ajna-findings

2 stars 0 forks source link

Extraordinary proposals can receive more tokens than eligible #376

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/sherlock-audit/2023-01-ajna/blob/main/ecosystem-coordination/src/grants/base/ExtraordinaryFunding.sol#L85

Vulnerability details

Note: The issue https://github.com/ajna-finance/audits/blob/main/sherlock/Contest1.md#issue-m-5-extraordinary-proposals-can-receive-more-tokens-than-eligible still exists

Summary

Two extraordinary proposals proposed at similar times (while no other extraordinary proposals are executed) can request the same amount of tokens (proposal.tokensRequested). But if the second proposal is proposed after the first proposal is successfully funded and executed, the second proposal could not request the same amount of tokens. It would have to be lower due to ExtraordinaryFunding.sol#L85.

Vulnerability Detail

Proposing an extraordinary funding proposal with the ExtraordinaryFunding.proposeExtraordinary function verifies that the requested token amount is within (less than) a certain limit in ExtraordinaryFunding.sol#L85. The limit is based on the treasury Ajna token balance and decreases with an increasing number of funded extraordinary proposals. A maximum number of 10 extraordinary proposals can be funded.

If multiple extraordinary proposals are proposed while the number of funded proposals is unchanged, the limit for the tokens requested is the same for those proposals. At a later time, if those proposals pass voting, the proposals are executed and receive the requested (stale) token amount.

Impact

The requested token amount of an extraordinary proposal is not checked when executing if it's within the same limits imposed by L86.

If another extraordinary proposal was successfully funded and executed in the meantime of the 1 month voting period for the proposal, the limit for the requested token amount is already lower than the proposal.tokensRequested amount.

This means that if multiple extraordinary proposals are proposed at a similar time and pass voting, the tokensRequested amount is stale and potentially too much.

Proof of Concept

https://github.com/sherlock-audit/2023-01-ajna/blob/main/ecosystem-coordination/src/grants/base/ExtraordinaryFunding.sol#L85

    null(
        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();

        // store newly created proposal
        newProposal.proposalId      = proposalId_;
        newProposal.startBlock      = SafeCast.toUint128(block.number);
        newProposal.endBlock        = SafeCast.toUint128(endBlock_);
        newProposal.tokensRequested = totalTokensRequested;

        emit ProposalCreated(
            proposalId_,
            msg.sender,
            targets_,
            values_,
            new string[](targets_.length),
            calldatas_,
            block.number,
            endBlock_,
            description_
        );
    }

Tools Used

Manual review

Recommended Mitigation Steps

Consider re-checking the tokensRequested amount when executing an extraordinary proposal and make sure it's within the same or similar limits as when proposing.

Assessed type

Other

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #353

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid