ArkProject: NFT Bridge

Summary

The check for payload.length is performed after deposition of tokens to escrow, even if the length is incorrect the deposition will happen anyways which is not desirable.

Vulnerability Details

depositTokens function

In Starklane::depositTokens function the payload.length is checked here

link to code

payload.length must remain within the MAX_PAYLOAD_LENGTH range for correct operation of the function.

payload is calculated as:

link to code

Hence, the value of payload depends on req
req is a struct of type Request, which keeps track of the request information.

req depends on the input parameter of the deposit function.

link to code

payload depends on req and req in turn depends on input parameters.

link to code

Here, we can see the number of token ids would directly effect value of req.hash, which in turn is being used to calculate payload

Impact

So the check for payload.length should be performed before depositing into escrow. In case the payload.length exceeds the max limit the function will revert after depositing into escrow, again it will have to be withdrawn to recover the tokens.

Tools Used

Manual review

Recommendations

Perform the check on payload.length before calling _depositIntoEscrow(ctype, collectionL1, ids); so that only correct amount of tokens are deposited into escrow.