ArkProject: NFT Bridge

Title

Certain message can never be bridged if we build a message with values above the maximum felt252

Line of code

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L78

Vulnerability Details

The function Starklane::depositTokens is used to deposit tokens in escrow and initiates the transfer to Starknet. The input parameter ownerL2 is used as the new owner on the Starknet and it is validated using if (!Cairo.isFelt252(snaddress.unwrap(ownerL2)))

According to the Cairo Language documentation

Before sending messages between L1 and L2, you must remember that Starknet contracts, written in Cairo, can only understand serialized data. And serialized data is always an array of felt252. In Solidity we have uint256 type, and felt252 are approximately 4 bits smaller than uint256. So we have to pay attention to the values contained in the payload of the messages we are sending. If, on L1, we build a message with values above the maximum felt252, the message will be stuck and never consumed on L2.

Now the issue arises because the other two input parameters salt and ids also uint256 type which are not validated as felt256 in cairo is approximately 4 bits smaller than uint256.

Impact

If the users put salt and ids input params greater than felt256 in the function Bridge::depositTokens() then the function will fail and will never be bridged.

Tools Used

Manual Review

Recommendations

We recommend validating the stated input parameters in the function Bridge::depositTokens()so that it is not above felt256.