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

2 stars 0 forks source link

ProposalHash can be easily duplicated with same target, callData and nativeValue #501

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/cgp/governance/InterchainGovernance.sol#L73 https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/cgp/governance/InterchainGovernance.sol#L143 https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/cgp/governance/AxelarServiceGovernance.sol#L53

Vulnerability details

Impact

    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);
    }

proposalHash is calculated from target, callData, and nativeValue. These values can be same if the signer wants to call the same function with same data and value. Using this kind of hash prevents such operations.

Proof of Concept

PoC is straightforward.

Tools Used

Manual

Recommended Mitigation Steps

Add block.timestamp or salt to hash. Possible to use incremental ID too.

Assessed type

call/delegatecall

0xSorryNotSorry commented 1 year ago

The submission does not provide any demonstration of the issue, reasoning and code blocks. It's organized by Voting storage voting = votingPerTopic[signerEpoch][topic];

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid