code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Wrong implementation of hashProposal in GrantFund contract. For computing of proposalId of standardFunding and extraordinaryFundig is used specific prefix before descriptionHash_.

For standardFunding is used constant DESCRIPTION_PREFIX_HASH_STANDARD and for extraordinaryFundig is used DESCRIPTION_PREFIX_HASH_EXTRAORDINARY.

If the user calls hashProposal function he always will receive invalid proposalId because prefix is missed before descriptionHash_. In the scenario where user calls hashProposal and after that findMechanismOfProposal, which are marked as view, to receive information about the proposal type if it is Standard or Extraordinary will always revert with ProposalNotFound().

Tools Used

Mannual review

Recommended Mitigation Steps

    function hashProposal(
        address[] memory targets_,
        uint256[] memory values_,
        bytes[] memory calldatas_,
        bytes32 descriptionHash_
        FundingMechanism type_,
    ) external pure override returns (uint256 proposalId_) {
        if(type_ == FundingMechanism.Standard) {
            proposalId_ = _hashProposal(targets_, values_, calldatas_, keccak256(abi.encode(DESCRIPTION_PREFIX_HASH_STANDARD, descriptionHash_)));
        } else { // the type is Extraordinary
            proposalId_ = _hashProposal(targets_, values_, calldatas_, keccak256(abi.encode(DESCRIPTION_PREFIX_HASH_EXTRAORDINARY, descriptionHash_)));
        } 
    }

Assessed type

Other

c4-sponsor commented 1 year ago

MikeHathaway marked the issue as sponsor disputed

MikeHathaway commented 1 year ago

The logic was messy and has been improved in a PR, but the hashProposal function works correctly assuming user input of a hashed description string with prefix.

Picodes commented 1 year ago

Downgrading as Low as this doesn't qualify for Medium severity per https://docs.code4rena.com/awarding/judging-criteria/severity-categorization

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)

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