code-423n4 / 2023-07-axelar-findings

2 stars 0 forks source link

All co-signers pay during executeMultisigProposal #491

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/governance/AxelarServiceGovernance.sol#L48-L52 https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/auth/MultisigBase.sol#L59-L62

Vulnerability details

Impact

The executeMultisigProposal will receive payment from all cosigners when only one payment of nativeValue amount is required.

Proof of Concept

The executeMultisigProposal requires native payment and therefore requires the caller to pay an amount equivalent to nativeValue. The onlySigners however, will only allow the executeMultisigProposal function body to execute for the caller that reaches the votes threshold.

From AxelarServiceGovernance.sol

    function executeMultisigProposal(
        address target,
        bytes calldata callData,
        uint256 nativeValue
    ) external payable onlySigners {
        bytes32 proposalHash = keccak256(abi.encodePacked(target, callData, nativeValue));

        if (!multisigApprovals[proposalHash]) revert NotApproved();

        multisigApprovals[proposalHash] = false;

        _call(target, callData, nativeValue);

        emit MultisigExecuted(proposalHash, target, callData, nativeValue);
    }

From MultisigBase.sol

    modifier onlySigners() {
        if (!signers.isSigner[msg.sender]) revert NotSigner();

        bytes32 topic = keccak256(msg.data);
        Voting storage voting = votingPerTopic[signerEpoch][topic];

        // Check that signer has not voted, then record that they have voted.
        if (voting.hasVoted[msg.sender]) revert AlreadyVoted();

        voting.hasVoted[msg.sender] = true;

        // Determine the new vote count.
        uint256 voteCount = voting.voteCount + 1;

        // Do not proceed with operation execution if insufficient votes.
        if (voteCount < signers.threshold) {
            // Save updated vote count.
            voting.voteCount = voteCount;
            return;
        }

        // Clear vote count and voted booleans.
        voting.voteCount = 0;

        uint256 count = signers.accounts.length;

        for (uint256 i; i < count; ++i) {
            voting.hasVoted[signers.accounts[i]] = false;
        }

        emit MultisigOperationExecuted(topic);

        _;
    }

Tools Used

n/a

Recommended Mitigation Steps

Either refund the native amount sent until there are insufficient votes to pass the proposal, or partially refund in amount equal to (nativeAmount - nativeAmount / signers.threshold) so that at the reach of the threshold an amount equal to nativeAmount has been paid as a sum of partial amounts from each cosigner.

Assessed type

Payable

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #334

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid