ArkProject: NFT Bridge

Summary

Due to difference in Request structs in both sides, this creates failure in desirialisation of requests causing NFTs get locked forever

Vulnerability Details

The newOwners field in L1 Request is being used as uint256[] array. This field is being serialised using uint256ArraySerialize method. But in L2 the Request defines this field as Span<ContractAddress>.

Serialisation of these both types is different. uint256 is serialised as two felts. ContractAddress is serialised as single felt. Due to this, deserialisation of the request will fail on either side. This will result in NFTs being stuck in the bridge.

Lets say, a request originates from L2 with the new owners set.

newOwners: [0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266, 0x5fbdb2315678afecb367f032d93f642f64180aa3]

After serialised as array of felts: [2, 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266, 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266]

During consumption L1 receive this as

Array of uint256s: [2, 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266, 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266]

As the new owners field is uint256, uint256ArrayDeserialize method is called.

As first element is 2, there should be two uint256 values in the array. That means there needs to be 4 additional elements where there are only 2, So desirialisation fails.

This message can never be consumed. NFTs will get locked forever in the L2 bridge contract.

Same issue occurs when trying to send NFTs from L1 -> L2 as derialisation fails on L2 bridge

Impact

User will lose the NFTs forever

Tools Used

Manual review

Recommendations

Use snaddress[] newOwners instead of uint256[] newOwners