Any proposal made in `GrantFund` cannot be executed

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/d80daab705a066828ef1c5d9ba85f315c7c932db/ajna-grants/src/grants/base/Funding.sol#L52-L66

Vulnerability details

Impact

Any proposal that has been proposed via proposeStandard or proposeExtraordinary cannot be executed via executeStandar or executeExtraordinary, because Funding._execute function will try to send ETH to ERC20.transfer function, which is not payable.

Proof of Concept

There are 2 ways to create proposals on GrantFund:

• StandardFunding.proposeStandard
• ExtraordinaryFunding.proposeExtraordinary

Both of these functions use Funding._validateCallDatas function that verifies if the input values are correct.

This function verifies that the targets_[i] address is ajnaTokenAddress(Funding L125), checks values_[i] != 0 , and checks the calldata starts with transfer(address,uint256) signature (Funding L115)

            ...
            if (targets_[i] != ajnaTokenAddress || values_[i] != 0) revert InvalidProposal(); 

            ...
            if (selector != bytes4(0xa9059cbb)) revert InvalidProposal();
            ...

This means that every call under one proposal is always called to ajnaTokenAddress with transfer(address,uint) with values_[i] > 0

Same as with creating proposals - there are 2 functions, to execute a proposal on GrantFund:

• StandardFunding.executeStandard
• ExtraordinaryFunding.executeExtraordinary

Both of these execute functions use Funding._execute

        _execute(proposalId_, targets_, values_, calldatas_);

    function _execute(
        uint256 proposalId_,
        address[] memory targets_,
        uint256[] memory values_,
        bytes[] memory calldatas_
    ) internal {
        // use common event name to maintain consistency with tally
        emit ProposalExecuted(proposalId_);

        string memory errorMessage = "Governor: call reverted without message";
        for (uint256 i = 0; i < targets_.length; ++i) {
            (bool success, bytes memory returndata) = targets_[i].call{value: values_[i]}(calldatas_[i]);
            Address.verifyCallResult(success, returndata, errorMessage);
        }
    }

The call function inside Funding._execute function uses values_[i] as msg.value. The issue is that every call is going to be a ajnaToken.transfer(address,uint256) and AjnaToken contract transfer function is not payable.

This means that every call made under the proposal will fail, because Funding._execute will try to call ajnaToken.transfer with values_[i] as msg.value (which is not 0), but ajnaToken.transfer function is not payable

Tools Used

Manual Review

Recommended Mitigation Steps

Since there is no way to send ETH to GrantFund (no fallback/receive function and no payable functions) and ajnaToken.transfer is not a payable function, I recommend removing values_ from functions where it is used.

Assessed type

ERC20

[c4-judge]

Picodes marked the issue as duplicate of #280

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

Picodes marked the issue as grade-b