Era
ZKsync
FoundryLayer 2
500,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

`L1Nullifier._parseL2WithdrawalMessage` - incorrect length check on `_l2ToL1message`

bladesec

Summary

In the _parseL2WithdrawalMessage function of the L1Nullifier contract, there is an incorrect length check on the _l2ToL1message byte array when the bytes4(functionSignature) equals IAssetRouterBase.finalizeDeposit.selector. The expected message length should be 68 bytes, but it is incorrectly checked as 36 bytes.

Vulnerability Details

_parseL2WithdrawalMessage function of the L1Nullifier contract is used to parse the withdrawal message and returns withdrawal details.

The function performs an internal length check on _l2ToL1message based on the function signature. However, when the bytes4(functionSignature) equals IAssetRouterBase.finalizeDeposit.selector, the expected length is incorrectly checked as 36 instead of the correct value of 68, as it requires at least two variables:

  • originalChainId: Uint256

  • assetId: Bytes32

Considering the variables required, the total expected byte length adds up to 4 + 32 + 32 = 68 bytes.

The implementation of the _parseL2WithdrawalMessage function is provided below, highlighting the length check and the process of reading variables:

function _parseL2WithdrawalMessage(
uint256 _chainId,
bytes memory _l2ToL1message
) internal returns (bytes32 assetId, bytes memory transferData) {
// Please note that there are three versions of the message:
// 1. The message that is sent from `L2BaseToken` to withdraw base token.
// 2. The message that is sent from L2 Legacy Shared Bridge to withdraw ERC20 tokens or base token.
// 3. The message that is sent from L2 Asset Router to withdraw ERC20 tokens or base token.
​
uint256 amount;
address l1Receiver;
​
(uint32 functionSignature, uint256 offset) = UnsafeBytes.readUint32(_l2ToL1message, 0);
if (bytes4(functionSignature) == IMailbox.finalizeEthWithdrawal.selector) {
...
} else if (bytes4(functionSignature) == IL1ERC20Bridge.finalizeWithdrawal.selector) {
...
} else if (bytes4(functionSignature) == IAssetRouterBase.finalizeDeposit.selector) {
// The data is expected to be at least 36 bytes long to contain assetId.
>> if (_l2ToL1message.length < 36) {
revert WrongMsgLength(36, _l2ToL1message.length);
}
// slither-disable-next-line unused-return
>> (, offset) = UnsafeBytes.readUint256(_l2ToL1message, offset); // originChainId, not used for L2->L1 txs
>> (assetId, offset) = UnsafeBytes.readBytes32(_l2ToL1message, offset);
transferData = UnsafeBytes.readRemainingBytes(_l2ToL1message, offset);
} else {
revert InvalidSelector(bytes4(functionSignature));
}
}

As mentioned in UnsafeBytes.sol, the byte length should be correctly validated before utilizing any functions from UnsafeBytes:

* @dev WARNING!
* 1) Functions don't check the length of the bytes array, so it can go out of bounds.
* The user of the library must check for bytes length before using any functions from the library!

Impact

Without a proper length check, the function risks reverting with an OutOfBounds error instead of the intended WrongMsgLength error, revealing a flaw in error reporting logic and creating debugging challenges.
Furthermore, if the length exceeds 36, attempting to access bytes beyond this point could result in unpredictable runtime behavior, introducing unexpected and potentially harmful system outcomes.

Tools Used

Manual Review

Recommendations

Validate _l2ToL1message.length against 68 instead of 36.

...
} else if (bytes4(functionSignature) == IAssetRouterBase.finalizeDeposit.selector) {
// The data is expected to be at least 36 bytes long to contain assetId.
- if (_l2ToL1message.length < 36) {
+ if (_l2ToL1message.length < 68) {
revert WrongMsgLength(36, _l2ToL1message.length);
}
// slither-disable-next-line unused-return
...
Updates

Lead Judging Commences

inallhonesty Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Inadequate length check for IAssetRouterBase.finalizeDeposit.Selector in function _parseL2WithdrawalMessage

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.