Lack of input validation
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 thatexecutionState[_settlementNonce]
is equal toSTATUS_READY
. Performing this check can save gas and reduce commission fees by reverting invalid requests early in theretrySettlement
function rather than later in thelzReceiveNonBlocking
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
andgetPortStrategyTokenDebt[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 thatgetPortStrategyTokenDebt[_strategy][_token]
is not zero value. This will prevent futile transactions and calls of arbitrary user's contracts.The
callOutAndBridge
andcallOutAndBridgeMultiple
functions of the RootBridgeAgent contract lacks a check that the_settlementOwnerAndGasRefundee
address is not zero.The
lzReceiveNonBlocking
function of the RootBridgeAgent contract forretrySettlement
case does not verify thatsettlement.status
is notSTATUS_FAILED
. This check will happen only after theIRouter(rootRouterAddress).executeRetrySettlement
call, which triggersIBridgeAgent(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 notSTATUS_SUCCESS
. However, a more precise approach would be to check that the status isSTATUS_FAILED
, as this indicates the settlement is ready for the redeem action.The
bridgeIn
andbridgeInMultiple
of the BranchPort contract lack alock
modifier against reentrancy attacks.The
bridgeToRoot
,bridgeToBranch
,bridgeToRootFromLocalBranch
,bridgeToLocalBranchFromRoot
,burnFromLocalBranch
, andmintToLocalBranch
functions of the RootPort contract lack alock
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↗.