Summary
Several functions within the ethereum::src::Bridge::Starklane contract invoke external contract functions but fail to utilize their return values, leading to potential information loss or increased risk.
The depositTokens function calls sendMessageToL2 on the IStarknetMessaging contract, which returns a tuple (bytes32, uint256) representing the hash of the message sent and its nonce.
The startRequestCancellation function calls startL1ToL2MessageCancellation, also on IStarknetMessaging, which returns a bytes32 representing the hash of the message being canceled.
The cancelRequest function invokes cancelL1ToL2Message, returning a bytes32 hash of the canceled message.
In all these instances, failing to capture and use the return values could lead to incorrect assumptions about the state of the contract or transaction, making it harder to debug or verify their execution.
Vulnerability Details
depositTokens function:
Interface of the sendMessageToL2 function, which returns the hash of the message and its nonce:
Sends a message to an L2 contract.
This function is payable, the payed amount is the message fee.
Returns the hash of the message and the nonce of the message.
*/
function sendMessageToL2(
uint256 toAddress,
uint256 selector,
uint256[] calldata payload
@> ) external payable returns (bytes32, uint256);
The code that calls sendMessageToL2 function in ethereum::src::Bridge::depositTokens doesn't utilize any returned values:
@> IStarknetMessaging(_starknetCoreAddress).sendMessageToL2{value: msg.value}(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload
);
startRequestCancellation function:
Interface of the startL1ToL2MessageCancellation function:
Starts the cancellation of an L1 to L2 message.
A message can be canceled messageCancellationDelay() seconds after this function is called.
Note: This function may only be called for a message that is currently pending and the caller
must be the sender of the that message.
*/
function startL1ToL2MessageCancellation(
uint256 toAddress,
uint256 selector,
uint256[] calldata payload,
uint256 nonce
@> ) external returns (bytes32);
Implementation of the startL1ToL2MessageCancellation function in the StarknetMessaging contract, which returns the message hash:
function startL1ToL2MessageCancellation(
uint256 toAddress,
uint256 selector,
uint256[] calldata payload,
uint256 nonce
) external override returns (bytes32) {
emit MessageToL2CancellationStarted(msg.sender, toAddress, selector, payload, nonce);
bytes32 msgHash = getL1ToL2MsgHash(toAddress, selector, payload, nonce);
uint256 msgFeePlusOne = l1ToL2Messages()[msgHash];
require(msgFeePlusOne > 0, "NO_MESSAGE_TO_CANCEL");
l1ToL2MessageCancellations()[msgHash] = block.timestamp;
@> return msgHash;
}
The code that calls startL1ToL2MessageCancellation function in ethereum::src::Bridge::startRequestCancellation doesn't utilize its returned value:
@> IStarknetMessaging(_starknetCoreAddress).startL1ToL2MessageCancellation(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload,
nonce
);
cancelRequest function:
Interface of the cancelL1ToL2Message function:
Cancels an L1 to L2 message, this function should be called at least
messageCancellationDelay() seconds after the call to startL1ToL2MessageCancellation().
A message may only be cancelled by its sender.
If the message is missing, the call will revert.
Note that the message fee is not refunded.
*/
function cancelL1ToL2Message(
uint256 toAddress,
uint256 selector,
uint256[] calldata payload,
uint256 nonce
@> ) external returns (bytes32);
Implementation of the cancelL1ToL2Message function in the StarknetMessaging contract, which returns the message hash:
function cancelL1ToL2Message(
uint256 toAddress,
uint256 selector,
uint256[] calldata payload,
uint256 nonce
) external override returns (bytes32) {
emit MessageToL2Canceled(msg.sender, toAddress, selector, payload, nonce);
bytes32 msgHash = getL1ToL2MsgHash(toAddress, selector, payload, nonce);
uint256 msgFeePlusOne = l1ToL2Messages()[msgHash];
require(msgFeePlusOne != 0, "NO_MESSAGE_TO_CANCEL");
uint256 requestTime = l1ToL2MessageCancellations()[msgHash];
require(requestTime != 0, "MESSAGE_CANCELLATION_NOT_REQUESTED");
uint256 cancelAllowedTime = requestTime + messageCancellationDelay();
require(cancelAllowedTime >= requestTime, "CANCEL_ALLOWED_TIME_OVERFLOW");
require(block.timestamp >= cancelAllowedTime, "MESSAGE_CANCELLATION_NOT_ALLOWED_YET");
l1ToL2Messages()[msgHash] = 0;
@> return (msgHash);
}
The code that calls cancelL1ToL2Message function in ethereum::src::Bridge::cancelRequest doesn't utilize its returned value:
@> IStarknetMessaging(_starknetCoreAddress).cancelL1ToL2Message(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload,
nonce
);
Impact
The returned data is ignored, potentially leading to scenarios where important transactional details are lost.
Tools Used
Manual Review
Recommendations
Capture and use the return values of those function. For example, by using them in emitting events:
depositTokens function:
- IStarknetMessaging(
+ (bytes32 messageHash, uint256 messageNonce) = IStarknetMessaging(
_starknetCoreAddress
).sendMessageToL2{value: msg.value}(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload
);
emit DepositRequestInitiated(
req.hash,
block.timestamp,
payload,
+ messageHash,
+ messageNonce
);
startRequestCancellation function:
- IStarknetMessaging(_starknetCoreAddress).
+ bytes32 messageHash = IStarknetMessaging(_starknetCoreAddress)
.startL1ToL2MessageCancellation(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload,
nonce
);
Request memory req = Protocol.requestDeserialize(payload, 0);
- emit CancelRequestStarted(req.hash, block.timestamp);
+ emit CancelRequestStarted(req.hash, block.timestamp, messageHash);
cancelRequest function:
- IStarknetMessaging(_starknetCoreAddress).cancelL1ToL2Message(
+ bytes32 messageHash = IStarknetMessaging(_starknetCoreAddress).cancelL1ToL2Message(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload,
nonce
);
Request memory req = Protocol.requestDeserialize(payload, 0);
_cancelRequest(req);
- emit CancelRequestCompleted(req.hash, block.timestamp);
+ emit CancelRequestCompleted(req.hash, block.timestamp, messageHash);
