Assessment reports>Maia DAO Ulysses Protocol>Low findings>Lack of input validation
Category: Coding Mistakes

Lack of input validation

Low Severity
Low Impact
Low Likelihood

Description

The audit has identified several functions across various contracts that currently lack essential input-validation checks. Implementing these checks can significantly improve the robustness and security of the protocol. Below is the list of functions:

  • The sweep function of the RootPort contract lacks a check that _recipient is not zero address.

  • The retrySettlement function of the BranchBridgeAgent contract lacks a check that executionState[_settlementNonce] is equal to STATUS_READY. Performing this check can save gas and reduce commission fees by reverting invalid requests early in the retrySettlement function rather than later in the lzReceiveNonBlocking function after completing the transferring of two cross-chain messages.

  • The replenishReserves(address _token, uint256 _amount) function of the BranchPort contract lacks a check that _amount and getPortStrategyTokenDebt[msg.sender][_token] are not zero values. This will prevent futile transactions and calls of arbitrary user's contracts.

  • The replenishReserves(address _strategy, address _token) function of the BranchPort contract lacks a check that getPortStrategyTokenDebt[_strategy][_token] is not zero value. This will prevent futile transactions and calls of arbitrary user's contracts.

  • The callOutAndBridge and callOutAndBridgeMultiple functions of the RootBridgeAgent contract lacks a check that the _settlementOwnerAndGasRefundee address is not zero.

  • The lzReceiveNonBlocking function of the RootBridgeAgent contract for retrySettlement case does not verify that settlement.status is not STATUS_FAILED. This check will happen only after the IRouter(rootRouterAddress).executeRetrySettlement call, which triggers IBridgeAgent(bridgeAgentAddress).retrySettlement. But it is better to perform critical checks as early as possible in the call stack.

  • The redeemSettlement function in the RootBridgeAgent contract verifies that the settlement status is not STATUS_SUCCESS. However, a more precise approach would be to check that the status is STATUS_FAILED, as this indicates the settlement is ready for the redeem action.

  • The bridgeIn and bridgeInMultiple of the BranchPort contract lack a lock modifier against reentrancy attacks.

  • The bridgeToRoot, bridgeToBranch, bridgeToRootFromLocalBranch, bridgeToLocalBranchFromRoot, burnFromLocalBranch, and mintToLocalBranch functions of the RootPort contract lack a lock modifier against reentrancy attacks.

Impact

If important input parameters are not checked, it can result in functionality issues and unnecessary gas usage and can even be the root cause of critical problems. It is crucial to properly validate input parameters to ensure the correct execution of a function and to prevent unintended consequences.

Recommendations

Consider adding require statements and necessary checks to the above functions.

Remediation

This issue has been acknowledged by Maia DAO, and a fix was implemented in commit bf16debc.

Zellic © 2024Back to top ↑