c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/MulticallRootRouter.sol#L127-L477

Vulnerability details

Impact

Three function Ids are not supported, namely 0x04, 0x05, 0x06, which goes against the docs since an attempt to execute this would always lead to a reversion

Proof of Concept

The intro to the Multicall Root Router Contract is that, asides the fact that this contract is the router implementation for interfacing with 3rd party dApps, the cross chain messaging func Ids that are supported are also clearly stated as can be seen here

 *         CROSS-CHAIN MESSAGING FUNCIDs
 *         -----------------------------
 *         FUNC ID      | FUNC NAME
 *         -------------+---------------
 *         0x01         | multicallNoOutput
 *         0x02         | multicallSingleOutput
 *         0x03         | multicallMultipleOutput
 *         0x04         | multicallSignedNoOutput
 *         0x05         | multicallSignedSingleOutput
 *         0x06         | multicallSignedMultipleOutput
 *

Crux of this issue is that going through the contract we can only see instances of supports for the first three IDs, namely 0x01, 0x02, 0x03, as seen from all the functions listed below:

execute()
executeSigned()
executeSignedDepositSingle()
executeSignedDepositMultiple()

Do note that all four functions aforementioned all have the else block below in the case where the func Id is not within 0x01..0x03

            /// UNRECOGNIZED FUNC ID
        } else {
            revert UnrecognizedFunctionId();
        }

The above claim is proven here

Tool used

Manual Review

Recommended Mitigation Steps

Extend support for the 0x04, 0x05, 0x06 IDs or or update the documentation to clearly indicate the current lack of support for these function IDs, noting the expected reversion behavior if they are accessed.

Assessed type

Context

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as duplicate of #757

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as sufficient quality report

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)