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

Nonce is not included in message consumption from L2, this can cause replay attack or hash collision when users consume messages from L2

0xdemon

Summary

Nonce is not included in message consumption from L2, this can cause replay attacks or hash collisions when users consume messages from L2

Vulnerability Details

When the user calls the depositTokens function, it contains a function to send a message from L1 to L2 with the sendMessageToL2 function. This function has a return of msg.hash and nonce. Then when the user wants to withdraw NFT from L2 with the withdrawTokens function, the _consumeMessageStarknet function in the Messaging.sol contract is called.

function _consumeMessageStarknet(
IStarknetMessaging starknetCore,
snaddress fromL2Address,
uint256[] memory request
)
internal
{
// Will revert if the message is not consumable.
bytes32 msgHash = starknetCore.consumeMessageFromL2(
snaddress.unwrap(fromL2Address),
request
);
​
// If the message were configured to be withdrawn with auto method,
// starknet method is denied.
if (_autoWithdrawn[msgHash] != WITHDRAW_AUTO_NONE) {
revert WithdrawMethodError();
}
}
​

As can be seen from the code above, there is no nonce checking and the use of nonce as msg.hash. This can cause a replay attack if in the future ArkNFT will be deployed on a different chain or hash collision.

Impact

This can cause replay attack or hash collision when users consume messages from L2

Tools Used

Manual Review

Recommended Mitigation

Make sure the user who owns the Request consumes the message from L2 according to the transaction nonce of the message sent from L1 to L2. This can be done by adding a variable to the Request struct and validating it when calling the _consumeMessageStarknet function.

function depositTokens(
uint256 salt,
address collectionL1,
snaddress ownerL2,
uint256[] calldata ids,
bool useAutoBurn
)
external
payable
{
......
// adding nonce to request struct
(, uint256 req.nonce) = IStarknetMessaging(_starknetCoreAddress).sendMessageToL2{value: msg.value}(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload
);
}
function withdrawTokens(
uint256[] calldata request
)
external
payable
returns (address)
{
......
_consumeMessageStarknet(_starknetCoreAddress, _starklaneL2Address, request, req.nonce);
......
}
function _consumeMessageStarknet(
IStarknetMessaging starknetCore,
snaddress fromL2Address,
uint256[] memory request
uint256 nonce
)
internal
{
bytes32 msgHash = keccak256(
abi.encodePacked(starknetCore.consumeMessageFromL2(
snaddress.unwrap(fromL2Address),
request), nonce)
);
......
}
​
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

invalid-replay-attack-hash-not-stored-nonce-not-used

There is no impact here: Transaction cannot be replayed because the blockchain use the nonce in the signature. Hash is computed on-chain. Using or trying to have the same hash mean you need to buy the token, and they will be sent to their origin owner. Why an attacker would buy tokens to give them back ? No real impact.

Support

FAQs

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