code-423n4 / 2023-09-maia-findings

25 stars 17 forks source link

Incorrect functionID will not trigger fallback #882

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L1090

Vulnerability details

Impact

When encoding a payload for settlement of multiple tokens, the fallback flag is not set when it should be. This will cause no fallback to be triggered even though the user has paid enough to cover the additional costs that are required.

Proof of Concept

In RootBridgeAgent._createSettlementMultiple the function ID which contains the fallback flag as first bit is encoded like this:

_hasFallbackToggled ? bytes1(0x02) & 0x0F : bytes1(0x02),

This bitwise AND of 0x02 with 0x0F will yield 0x02 (same as the no fallback case). Hence no fallback will be triggered in case of failure on the branch chain:

function lzReceiveNonBlocking(address _endpoint, bytes calldata _srcAddress, bytes calldata _payload)
    public
    override
    requiresEndpoint(_endpoint, _srcAddress)
{
    //Save Action Flag
    bytes1 flag = _payload[0] & 0x7F; //remove leftmost bit
    ...
    } else if (flag == 0x02) {
    ...
    _execute(
        _payload[0] == 0x82, //hasFallback
    ...

Minimal PoC with foundry to showcase 0x02 & 0x0F == 0x02:

pragma solidity ^0.8.13;

import "forge-std/Test.sol";

contract BitOperationsTest is Test {

    function testBitwiseAnd() public{
        assert(bytes1(0x02) & 0x0F == 0x02);
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

Looking at how RootBridgeAgent._createSettlement determines the first byte:

_hasFallbackToggled ? bytes1(0x81) : bytes1(0x01),

This should be done analoguously for _createSettlementMultiple

_hasFallbackToggled ? bytes1(0x82) : bytes1(0x02),

Assessed type

Other

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as primary issue

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as sufficient quality report

c4-sponsor commented 1 year ago

0xBugsy (sponsor) confirmed

c4-judge commented 1 year ago

alcueca marked the issue as satisfactory

c4-judge commented 1 year ago

alcueca marked issue #397 as primary and marked this issue as a duplicate of 397