ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Unchecked Message Hash Addition for Auto-Withdrawal

akay

Summary

The addMessageHashForAutoWithdraw function in the StarklaneMessaging contract allows the owner to add message hashes for auto-withdrawal without verifying the validity or existence of the corresponding messages. This could potentially lead to the addition of arbitrary or non-existent message hashes.

Vulnerability Details

The addMessageHashForAutoWithdraw function takes a uint256 msgHash as input, converts it to bytes32, and adds it to the _autoWithdrawn mapping if it's not already present but the function doesn't verify that the hash corresponds to an actual message sent from L2 or that the message content is valid and follows the expected format or that the message hasn't been tampered with.

function addMessageHashForAutoWithdraw(
uint256 msgHash
)
external
payable
onlyOwner
{
bytes32 hash = bytes32(msgHash);
​
if (_autoWithdrawn[hash] != WITHDRAW_AUTO_NONE) {
revert WithdrawMethodError();
}
​
_autoWithdrawn[hash] = WITHDRAW_AUTO_READY;
emit MessageHashAutoWithdrawAdded(hash);
}

The only check performed is to ensure the hash hasn't already been added for auto-withdrawal.

Impact

Addition of non-existent message hashes, allowing for unauthorized withdrawals. Also, manipulation of the auto-withdrawal system by adding arbitrary hashes. It can also lead to potential financial losses if the auto-withdrawal process is exploited.

Tools Used

Manual code review

Recommendations

Implement a verification mechanism to ensure the message hash corresponds to an actual message sent from L2. This could involve interacting with the Starknet core contract or maintaining a separate record of valid messages. Also add checks to validate the content and format of the message before adding its hash for auto-withdrawal.

function addMessageHashForAutoWithdraw(
uint256 msgHash,
// Additional parameters to verify the message
address fromL2Address,
uint256[] memory messageContent
)
external
payable
onlyOwner
{
bytes32 hash = bytes32(msgHash);
​
// Verify the message exists and is valid
require(verifyMessageFromL2(fromL2Address, messageContent), "Invalid message");
​
// Verify the hash matches the message content
require(hash == keccak256(abi.encodePacked(fromL2Address, address(this), messageContent)), "Hash mismatch");
​
if (_autoWithdrawn[hash] != WITHDRAW_AUTO_NONE) {
revert WithdrawMethodError();
}
​
// Start a challenge period
_pendingAutoWithdrawals[hash] = block.timestamp + CHALLENGE_PERIOD;
​
emit MessageHashAutoWithdrawPending(hash, fromL2Address, messageContent);
}
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Support

FAQs

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