010.md

Steep Orange Lynx

Medium

Unhandled Callback Failure in dropMessage

Summary

The dropMessage function in the L1CrossDomainMessenger contract lacks error handling for the onDropMessage callback to the _from address. This can lead to untracked failures, resulting in inconsistent state and potential loss of critical message drop notifications.

Vulnerability Detail

The vulnerability is rooted in the direct call to onDropMessage on the _from address without any error handling mechanism. If the call fails, the function does not capture or log the failure, leading to silent errors.

241:     function dropMessage(
242:         address _from,
243:         address _to,
244:         uint256 _value,
245:         uint256 _messageNonce,
246:         bytes memory _message
247:     ) external override whenNotPaused notInExecution {
248:         // The criteria for dropping a message:
---
260:         address _messageQueue = messageQueue;
261: 
---
263:         bytes memory _xDomainCalldata = _encodeXDomainCalldata(_from, _to, _value, _messageNonce, _message);
264:         bytes32 _xDomainCalldataHash = keccak256(_xDomainCalldata);
---
264:         bytes32 _xDomainCalldataHash = keccak256(_xDomainCalldata);
265:         require(messageSendTimestamp[_xDomainCalldataHash] > 0, "Provided message has not been enqueued");
---
265:         require(messageSendTimestamp[_xDomainCalldataHash] > 0, "Provided message has not been enqueued");
266: 
---
268:         require(!isL1MessageDropped[_xDomainCalldataHash], "Message already dropped");
269: 
---
271:         uint256 _lastIndex = replayStates[_xDomainCalldataHash].lastIndex;
272:         if (_lastIndex == 0) _lastIndex = _messageNonce;
273: 
---
276:         while (true) {
277:             IL1MessageQueue(_messageQueue).dropCrossDomainMessage(_lastIndex);
278:             _lastIndex = prevReplayIndex[_lastIndex];
279:             if (_lastIndex == 0) break;
280:             unchecked {
281:                 _lastIndex = _lastIndex - 1;
282:             }
283:         }
---
285:         isL1MessageDropped[_xDomainCalldataHash] = true;
286: 
---
288:         xDomainMessageSender = Constants.DROP_XDOMAIN_MESSAGE_SENDER;
289:@=>      IMessageDropCallback(_from).onDropMessage{value: _value}(_message);
---
291:         xDomainMessageSender = Constants.DEFAULT_XDOMAIN_MESSAGE_SENDER;
292:     }

If the function execution, the current implementation does not handle these failures.

Impact

Failure in onDropMessage can lead to an inconsistent state where the message is considered dropped in the L1 queue but not acknowledged by the _from contract.

Code Snippet

https://github.com/sherlock-audit/2024-08-morphl2/blob/main/morph/contracts/contracts/l1/L1CrossDomainMessenger.sol#L241-L292

Tool used

Manual Review

Recommendation

Implement error handling for the onDropMessage call. This can be done using a try-catch block to capture and manage potential failures.

function dropMessage(
    address _from,
    address _to,
    uint256 _value,
    uint256 _messageNonce,
    bytes memory _message
) external override whenNotPaused notInExecution {
    // ... [existing code for validation and dropping message] ...

    // set execution context
    xDomainMessageSender = Constants.DROP_XDOMAIN_MESSAGE_SENDER;
-   IMessageDropCallback(_from).onDropMessage{value: _value}(_message);
    // Handle potential failure in onDropMessage
+   try IMessageDropCallback(_from).onDropMessage{value: _value}(_message) {
        // Successfully called onDropMessage
+   } catch {
        // Log the failure for auditing and debugging
+       emit DropMessageFailed(_from, _messageNonce, _message);
    }

    // clear execution context
    xDomainMessageSender = Constants.DEFAULT_XDOMAIN_MESSAGE_SENDER;
}