Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect argument order in CrossChainHandleResult event emission leads to incorrect cross-chain message finalization #528

Open
howlbot-integration bot opened this issue Sep 15, 2024 · 2 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-129 grade-a QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_18_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-08-chakra/blob/abef77d95866f2fec93270491fc5abc9ab14f86d/solidity/settlement/contracts/ChakraSettlement.sol#L238-L241

Vulnerability details

Impact

When a cross-chain ERC20 transfer is initiated, the destination ChakraSettlement contract processes the message via the receive_cross_chain_msg function and emits a CrossChainHandleResult event. This event is crucial for Chakra validators, as they rely on its arguments to correctly call ChakraSettlement::receive_cross_chain_callback on the source chain to finalize the message.

The issue lies in the incorrect ordering of arguments in the CrossChainHandleResult event. This misordering causes the validators to pass incorrect values to the receive_cross_chain_callback function, leading to improper message finalization:

// cross chain handle result emit by receive side
event CrossChainHandleResult(
    uint256 indexed txid,
    CrossChainMsgStatus status,
    string from_chain,
    string to_chain,
    address from_handler,
    uint256 to_handler,
    PayloadType payload_type
);

function receive_cross_chain_msg(
    uint256 txid,
    string memory from_chain,
    uint256 from_address,
    uint256 from_handler,
    address to_handler,
    PayloadType payload_type,
    bytes calldata payload,
    uint8 sign_type, // validators signature type /  multisig or bls sr25519
    bytes calldata signatures // signature array
) external {
    // --SNIP

    emit CrossChainHandleResult(
        txid,
        status,
>>>        contract_chain_name, // @audit this should be `from_chain`
>>>        from_chain,          // @audit this should be `contract_chain_name`
>>>        address(to_handler), // @audit this should be `from_handler`
>>>        from_handler,        // @audit this should be `to_handler`
        payload_type
    );
}

As shown above, the receive_cross_chain_msg function incorrectly sets the arguments in the CrossChainHandleResult event. When Chakra validators capture this event and use it to call receive_cross_chain_callback on the source chain, the wrong values are passed to the function. For instance, the from_chain argument in the event emission in receive_cross_chain_callback ends up referring to the source chain itself, rather than the intended destination chain that handled the receive message.

Tools Used

Manual Review

Recommended Mitigation Steps

Ensure that the arguments in the CrossChainHandleResult event are correctly ordered:

function receive_cross_chain_msg(
   uint256 txid,
   string memory from_chain,
   uint256 from_address,
   uint256 from_handler,
   address to_handler,
   PayloadType payload_type,
   bytes calldata payload,
   uint8 sign_type, // validators signature type /  multisig or bls sr25519
   bytes calldata signatures // signature array
) external {
   // --SNIP

   emit CrossChainHandleResult(
       txid,
       status,
-       contract_chain_name, 
-       from_chain,          
-       address(to_handler), 
-       from_handler,    
+       from_chain,
+       contract_chain_name,
+       from_handler,
+       address(to_handler)    
      payload_type

   );
}

Assessed type

Other

@howlbot-integration howlbot-integration bot added 3 (High Risk) Assets can be stolen/lost/compromised directly 🤖_18_group AI based duplicate group recommendation bug Something isn't working duplicate-129 sufficient quality report This report is of sufficient quality labels Sep 15, 2024
howlbot-integration bot added a commit that referenced this issue Sep 15, 2024
@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Oct 24, 2024
@c4-judge
Copy link
Contributor

alex-ppg changed the severity to QA (Quality Assurance)

@c4-judge
Copy link
Contributor

alex-ppg marked the issue as grade-b

@C4-Staff C4-Staff reopened this Nov 1, 2024
@C4-Staff C4-Staff added grade-a and removed grade-b labels Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-129 grade-a QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_18_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

2 participants