NFTBridge
60,000 USDC
View results
Submission Details
Severity: medium
Invalid

Auto withdrawed messages `L2->L1` will get reverted when replayed

Vulnerability Details

When withdrawing from L2 to L1 there is a feature called auto withdraw where instead of consuming the msg from the Starknet messaging protocol, it is consumed automatically by the Bridge by Admins. Where they can add the message from L2 to be auto withdrawn in case it is not taken by the Starknet sequencer.

Messaging.sol#L46-L61

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

The Bridge owner can add msgHash into auto-withdraw messages if that message is not added.

When consuming the msg, we calculate its hash and if it was added into auto withdrew messages it will get consumed.

Messaging.sol#L69-L90

function _consumeMessageAutoWithdraw(
snaddress fromL2Address,
uint256[] memory request
) internal {
bytes32 msgHash = keccak256(
❌️ abi.encodePacked(
snaddress.unwrap(fromL2Address),
uint256(uint160(address(this))),
request.length,
request)
);
uint256 status = _autoWithdrawn[msgHash];
if (status == WITHDRAW_AUTO_CONSUMED) {
revert WithdrawAlreadyError();
}
_autoWithdrawn[msgHash] = WITHDRAW_AUTO_CONSUMED;
}

When consuming the message we calculate it using its inputs on L2, and then set that msg to WITHDRAW_AUTO_CONSUMED.

The problem is that this logic is correct if the msg is not replayable. i.e the msgHash cannot be replayed by implementing nonce or something. But this is not true.

When computing the hash we are computing it using the sender from L2, L1bridge, and the payload data.

The same L2sender can do more than one transaction from L2 to L1Bridge is always the same. and now we need to see if the payload data is the same or not.

Payload is the Hash of the Request object which consists of the following parameters.

struct Request {
felt252 header;
uint256 hash;
address collectionL1;
snaddress collectionL2;
address ownerL1;
snaddress ownerL2;
string name;
string symbol;
string uri;
uint256[] tokenIds;
uint256[] tokenValues;
string[] tokenURIs;
uint256[] newOwners;
}

This is the Request object in solidity which is the same in Starknet L2 Bridge. All the variables will be the same if the message sent from layer2 provides the same parameters (i.e he will send the NFT from L2 to L1 to the same address on L1). and now only the hash can achieve the uniqueness of the message hash.

The Hash also, is not unique where it is calculated using salt, collection, to_l1_address, and tokenIds. and as we said, all these things can be replayed in more than one message, and for the salt itself, it is written by the user which can be written as the value in the previous message he requested.

So in brief, the message from L2 can be Replayed and msgHash will be the same. Now what will happen if a message is autoWithdrawed and the same message is initiated before on L2.

First: The message can't be auto withdrawn as it is value in the mapping is CONSUMED and to add it to auto withdrawal, it should have auto withdraw to NONE not CONSUMED.

Second: The message will still be unable to get consumed by Starknet protocol, as when consuming the message from Starknet, the message should not be auto-withdrawn.

Messaging.sol#L112-L116

function _consumeMessageStarknet( ... ) 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();
}
}

We should keep in mind that replaying the message from Starknet to Ethereum is a normal thing is StarknetMessaging protocol, which supports replaying the messages more than one time without a problem.

This will result in preventing consuming the message, and withdrawing NFTs if the msgHash is the same.

This issue affects msgHash which was pointed to be autoWithdraw before or that is added to autoWithdraw.

Proof of Concept

  • MsgA was AutoWithdrawen message.

  • UserA initiated a message that is the same as MsgA.

  • The MsgA has the same hash as the msg initiated by UserA.

  • Starknet Messaging protocol accepted the message, and add it.

  • UserA called L1Bridge::withdrawTokens().

  • The tx reverted as that msgHash was auto-withdrawn before.

  • The NFTs in that message are lost and can't be withdrawn.

Impacts

  • Losing of NFTs in the messages that has a msgHash matchs on of the Autowithdrawed messages.

  • Unable to auto withdraw the same msgHash twice, which is an acceptable thing in StarknetMessaging protocol.

Tools Used

Manual Review

Recommendations

There are a lot of possible solutions for this issue, the best thing is to support the replaying of the msg from L2 the same as Starknet Messaging protocol works. But for simple mitigation, we can implement a function to reset the msg to NONE by admins. So if this thing occurs, the Admins can reset the msgHash into NONE in auto, which will allow consuming the message either by Starknet or auto.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

alqaqa Submitter
9 months ago
n0kto Lead Judge
8 months ago
n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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