Maia DAO - Ulysses - versiyonbir's results

Lines of code

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

Vulnerability details

ACCOUNT EXISTENCE CHECK FOR LOW LEVEL CALLS

MulticallRootRouter, appears to have a potential vulnerability related to account existence checks for low-level calls. The vulnerability may exist in the following functions:

1. _approveAndCallOut
2. _approveMultipleAndCallOut In these functions, there is a sequence of low-level calls to approve token transfers and then transfer tokens to another address. However, there are no checks to verify the existence or validity of the recipient address (recipient) and the bridge agent address (_bridgeAgentAddress) before performing these operations. Here's the code snippet where the vulnerability exists:

// In _approveAndCallOut
outputToken.safeApprove(_bridgeAgentAddress, amountOut);
IBridgeAgent(_bridgeAgentAddress).callOutAndBridge{value: msg.value}(
    payable(refundee),
    recipient,
    dstChainId,
    "",
    SettlementInput(outputToken, amountOut, depositOut),
    gasParams,
    true
);

// In _approveMultipleAndCallOut
for (uint256 i = 0; i < outputTokens.length;) {
    outputTokens[i].safeApprove(_bridgeAgentAddress, amountsOut[i]);
    unchecked {
        ++i;
    }
}
IBridgeAgent(_bridgeAgentAddress).callOutAndBridgeMultiple{value: msg.value}(
    payable(refundee),
    recipient,
    dstChainId,
    "",
    SettlementMultipleInput(outputTokens, amountsOut, depositsOut),
    gasParams,
    true
);

In both functions, the contract directly interacts with external addresses (recipient and _bridgeAgentAddress) without confirming if they are valid and exist. This could lead to potential issues such as:

1. Loss of Funds: If recipient or _bridgeAgentAddress is an invalid or non-contract address, it may lead to funds being sent to an unintended or nonexistent entity.
2. Reentrancy Attacks: Malicious contracts could potentially exploit this vulnerability to perform reentrancy attacks by having a fallback function or reentrancy attack within the target contract at the recipient address. To mitigate this vulnerability, it is advisable to add checks to ensure that recipient and _bridgeAgentAddress are valid and non-zero addresses before interacting with them. You can use the require statement to perform these checks. Additionally, consider using the "checks-effects-interactions" pattern to minimize reentrancy vulnerabilities. Here's an example of how to add checks for existence:

// In _approveAndCallOut
require(recipient != address(0), "Invalid recipient address");
require(_bridgeAgentAddress != address(0), "Invalid bridge agent address");

outputToken.safeApprove(_bridgeAgentAddress, amountOut);
IBridgeAgent(_bridgeAgentAddress).callOutAndBridge{value: msg.value}(
    payable(refundee),
    recipient,
    dstChainId,
    "",
    SettlementInput(outputToken, amountOut, depositOut),
    gasParams,
    true
);

// In _approveMultipleAndCallOut
require(recipient != address(0), "Invalid recipient address");
require(_bridgeAgentAddress != address(0), "Invalid bridge agent address");

for (uint256 i = 0; i < outputTokens.length;) {
    outputTokens[i].safeApprove(_bridgeAgentAddress, amountsOut[i]);
    unchecked {
        ++i;
    }
}
IBridgeAgent(_bridgeAgentAddress).callOutAndBridgeMultiple{value: msg.value}(
    payable(refundee),
    recipient,
    dstChainId,
    "",
    SettlementMultipleInput(outputTokens, amountsOut, depositsOut),
    gasParams,
    true
);

By adding these checks, you can ensure that the contract interacts only with valid and existing addresses, reducing the risk of potential vulnerabilities related to account existence.

Assessed type

call/delegatecall