Lines of code
<https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/Funding.sol#L52-L66&gt;

Vulnerability details

Impact

The Funding.sol contract’s _validateCallDatas function validates the targets, values, and calldatas parameters for a proposal but does not check if the proposal is valid based on the current state of the contract. For example, it does not check if the proposal has already been executed or if the proposer has sufficient voting power. This could allow a malicious user to execute proposals that have already been executed or to submit proposals without sufficient voting power, resulting in loss of funds or unfair decision-making.

If additional checks are not implemented in the contract code to ensure that proposals are valid before executing them, there could be several potential consequences.

Proposals that have already been executed could be executed again, Proposals submitted by addresses without sufficient voting power could be executed, Invalid proposals could be executed.

Proof of Concept

The _validateCallDatas function: Funding#L103-L141

function _validateCallDatas(
        address[] memory targets_,
        uint256[] memory values_,
        bytes[] memory calldatas_
    ) internal view returns (uint128 tokensRequested_) {

        // check params have matching lengths
        if (targets_.length == 0 || targets_.length != values_.length || targets_.length != calldatas_.length) revert InvalidProposal();

        for (uint256 i = 0; i < targets_.length;) {

            // check targets and values params are valid
            if (targets_[i] != ajnaTokenAddress || values_[i] != 0) revert InvalidProposal();

            // check calldata function selector is transfer()
            bytes memory selDataWithSig = calldatas_[i];

            bytes4 selector;
            assembly {
                selector := mload(add(selDataWithSig, 0x20))
            }
            if (selector != bytes4(0xa9059cbb)) revert InvalidProposal();

            // https://github.com/ethereum/solidity/issues/9439
            // retrieve tokensRequested from incoming calldata, accounting for selector and recipient address
            uint256 tokensRequested;
            bytes memory tokenDataWithSig = calldatas_[i];
            assembly {
                tokensRequested := mload(add(tokenDataWithSig, 68))
            }

            // update tokens requested for additional calldata
            tokensRequested_ += SafeCast.toUint128(tokensRequested);
            unchecked { ++i; }
        }
    }

The _execute function: Funding.sol#L52-L64

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 fact that the Funding contract does not check whether the proposal has already been executed or whether the proposer has sufficient voting power. This means that a proposal that has already been executed or a proposal submitted by an address without sufficient voting power could still be executed.

The _execute function and the _validateCallDatas function in this case. The _validateCallDatas function is responsible for validating the inputs of the proposal, including the targets, values, and calldatas arrays. The _execute function, on the other hand, takes these validated inputs and executes them.

However, the _validateCallDatas function only validates the inputs of the proposal, but it does not check whether the proposal is valid based on the current state of the contract. This means that a proposal that has already been executed or a proposal that was submitted by an address without sufficient voting power could still be executed.

As a result, the vulnerability in the _validateCallDatas function can be exploited by an attacker to bypass the checks that are supposed to ensure the proposal is valid, and then have the _execute function execute the proposal, resulting in unintended consequences for the contract.

Tools Used

vscode

Recommended Mitigation Steps

Additional checks should be implemented in the _validateCallDatas function and _execute function to ensure that proposals are valid based on the current state of the contract. The _validateCallDatas function should check if the proposal has already been executed and if the proposer has sufficient voting power to submit the proposal. The _execute function should also check if the proposal has already been executed before executing it.

Assessed type

Error

---

The text was updated successfully, but these errors were encountered:

All reactions