ArkProject: NFT Bridge

Summary

The Cairo library in the provided smart contract includes several functions for serializing and deserializing data types used in Cairo. However, the cairoStringUnpack function is vulnerable to incorrect string length handling, which can lead to buffer overflow issues and incorrect deserialization of strings.

Description

The cairoStringUnpack function in the Cairo library is vulnerable to insufficient buffer validation, leading to potential buffer overflow issues and incorrect deserialization of strings. This vulnerability could compromise the integrity of the NFT transfer process and potentially cause NFT loss and bridge instability.

Vulnerability Details

The vulnerability exists in the cairoStringUnpack function, which extracts a string from a buffer by concatenating parts of the buffer into a string. The issues are:

• The function does not properly handle cases where the buffer contains incomplete or malformed data.

• The function assumes that buffer length information is accurate, lacking sufficient bounds checking.

• If the length information is incorrect or if the buffer is truncated, it may lead to buffer overflows or incorrect data handling.

Impact

• Buffer Overflow: The function may read beyond allocated memory if the buffer contains fewer bytes than expected.

• Incorrect Data Handling: The function may produce incorrect strings if the buffer is not properly formatted or if the data length is misreported.

Exploit Scenario

1. Initial Setup: Bob initiates an NFT transfer from Ethereum (L1) to Starknet (L2).

2. Execution: The cairoStringUnpack function is called to deserialize the message on Starknet (L2).

3. Vulnerability Exploitation: If the message is malformed or shorter than expected, the function may read beyond the allocated memory, leading to memory corruption or unexpected behavior.

Impact on Users

• Bob: May experience failed or incorrect NFT transfers, leading to a negative user experience.

• Alice: Might receive incorrect NFT data, potentially affecting asset ownership and trust in the NFT Bridge service.

Test Case(add this to Cairo.t.sol)

Analysis

The test case fails with errors when handling malformed buffers, confirming that the function does not adequately handle buffer overflows and incorrect data.

Tools Used

• Solidity: Language for implementing the Cairo library.

• Forge: Testing framework used to create the test cases.

• Manual Analysis: Additional examination of code and behavior.

Recommendations

1. Add Length Checks: Implement checks to ensure buffer length is sufficient to prevent reading beyond its bounds.

   require(buf.length > offset + length / 32 + 1, "Buffer underflow");

2. Improved Error Handling: Include additional checks to catch cases where the buffer index might go out of bounds.

   require(index < buf.length, "Buffer index out of bounds");

3. Use Assertions for Validation: Utilize assert for internal logic checks to ensure correctness during development.

   // Inside cairoStringUnpack function

   assert(buf.length > 0);