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

2 stars 0 forks source link

findMechanismOfProposal could shadow an extraordinary proposal #368

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/GrantFund.sol#L36-L42 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/Funding.sol#L152-L159 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/GrantFund.sol#L45-L51

Vulnerability details

Impact

Alice creates an extraordinary proposal to request 10 million AJNA tokens to pay for something important. Mallory does not like the proposal and creates a standard proposal with the same arguments. The front end, which calls state() to view the state of any type of proposal, now returns the state of Mallory's standard proposal instead of Alice's extraordinary proposal.

Proof of Concept

The findMechanismOfProposal function will shadow an existing extraordinary proposal if a standard proposal with the same proposal ID exists. That is, the function will report that a given proposal ID corresponds to a standard proposal, even though an extraordinary proposal with the same ID exists.

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

function findMechanismOfProposal(
        uint256 proposalId_
    ) public view returns (FundingMechanism) {
        if (_standardFundingProposals[proposalId_].proposalId != 0)           return FundingMechanism.Standard;
        else if (_extraordinaryFundingProposals[proposalId_].proposalId != 0) return FundingMechanism.Extraordinary;
        else revert ProposalNotFound();
    }

Proposal IDs for both types of proposals are generated by hashing the proposal arguments, which are the same for both proposals

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

function _hashProposal(
        address[] memory targets_,
        uint256[] memory values_,
        bytes[] memory calldatas_,
        bytes32 descriptionHash_
    ) internal pure returns (uint256 proposalId_) {
        proposalId_ = uint256(keccak256(abi.encode(targets_, values_, calldatas_, descriptionHash_)));
    }

The findMechanismOfProposal function is also called from the state function, which reports the state of a given proposal by ID.

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

function state(
        uint256 proposalId_
    ) external view override returns (ProposalState) {
        FundingMechanism mechanism = findMechanismOfProposal(proposalId_);

        return mechanism == FundingMechanism.Standard ? _standardProposalState(proposalId_) : _getExtraordinaryProposalState(proposalId_);
    }

Depending on how the state view function is used, its use of the flawed findMechanismOfProposal function could cause problems in the front end or other smart contracts that integrate with the GrantFund contract.

Tools Used

Manual review

Recommended Mitigation Steps

Short term, redesign the findMechanismOfProposal function so that it does not shadow any proposal. For example, have the function return an array of two items that will indicate whether a standard and extraordinary proposal with that proposal ID exists.

Long term, consider all of the information that the front end and other integrating smart contracts might require to function correctly and design the corresponding view functions in the smart contracts to fulfill those requirements.

Assessed type

Other

MikeHathaway commented 1 year ago

Since we're using a constant prefix hash here: https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L62 it would be unrealistic for someone to be able to generate the same proposalId necessary for this to occur

Picodes commented 1 year ago

I agree with the previous comment

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Overinflated severity