c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/ffbe532c6f5224a55ce099b4016bd8806bdbc913/src/RootBridgeAgentExecutor.sol#L97 https://github.com/code-423n4/2023-09-maia/blob/ffbe532c6f5224a55ce099b4016bd8806bdbc913/src/RootBridgeAgentExecutor.sol#L243-L246

Vulnerability details

Impact

Users will not receive the Global HTokens they are supposed to get when bridging tokens from the branch chain to the root chain. The tokens end up being minted to the router, effectively causing a loss for the user.

Proof of Concept

When users want to bridge tokens from a branch chain to the root chain, they call the BranchBridgeAgent.callOutAndBridge function. This function locks the tokens in the BranchPort and sends a message to the root chain to initiate the bridging process.

In the process, the RootBridgeAgentExecutor contract then calls the executeWithDeposit function, which in turn calls _bridgeIn to mint the tokens. However, the recipient address (the first argument) is incorrectly set to the router's address. The _bridgeIn function should mint the tokens to the user's address, but due to the incorrect recipient address, the tokens are minted to the router instead.

    _bridgeIn(_router, dParams, _srcChainId);

https://github.com/code-423n4/2023-09-maia/blob/ffbe532c6f5224a55ce099b4016bd8806bdbc913/src/RootBridgeAgentExecutor.sol#L97

function _bridgeIn(address _recipient, DepositParams memory _dParams, uint16 _srcChainId) internal {
    //Request assets for decoded request.
    RootBridgeAgent(payable(msg.sender)).bridgeIn(_recipient, _dParams, _srcChainId);
}

https://github.com/code-423n4/2023-09-maia/blob/ffbe532c6f5224a55ce099b4016bd8806bdbc913/src/RootBridgeAgentExecutor.sol#L243-L246

In case there is no additional calldata, the funds is stuck in the router and user cannot have the global tokens for the deposited amonut in branch chain.

    // Check if there is additional calldata in the payload
    if (_payload.length > PARAMS_SETTLEMENT_OFFSET) {
        //Execute remote request
        IRouter(_router).executeSignedDepositSingle{value: msg.value}(
            _payload[PARAMS_SETTLEMENT_OFFSET:], dParams, _account, _srcChainId
        );
    }

Tools Used

Manual

Recommended Mitigation Steps

Change the first argument in the _bridgeIn function to the user's address instead of the router's address.

Assessed type

Error

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as sufficient quality report

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as primary issue

0xA5DF commented 1 year ago

Leaving open for sponsor to comment

0xBugsy commented 1 year ago

We believe this is a duplicate of #406

alcueca commented 1 year ago

As pointed elsewhere, Routers are not expected to hold assets between transactions and the user should specify what to do with the assets in the _params.

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)

alcueca commented 1 year ago

The sponsors should make the documentation clear in this aspect.