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

Improper Deserialization Logic in `SpanFeltTryIntoByteArray` can lead to Incorrect ByteArray Conversion (`byte_array_extra.cairo::SpanFeltTryIntoByteArray`)

Summary

Vulnerability Detail

The SpanFeltTryIntoByteArray implementation in the starknet/src/byte_array_extra.cairo contract is responsible for converting a span of felt252 elements into a ByteArray. This function is crucial for handling multiple felt252 elements and converting them into a single ByteArray representation. The function first checks if the span is empty or contains a single element, handling these cases appropriately. However, when the span contains more than one element, the function attempts to deserialize the entire span using Serde::deserialize(ref self). This approach is flawed because it does not correctly handle the conversion of multiple felt252 elements into a ByteArray. Instead, it tries to deserialize the span as a whole, which is not the intended behavior and can lead to incorrect deserialization and potential runtime errors.

Impact

The improper deserialization logic can lead to incorrect conversion of spans with multiple felt252 elements into a ByteArray. This can result in data corruption, unexpected behavior, and potential security vulnerabilities. Specifically, the function may return incorrect ByteArray objects, which can affect any downstream logic relying on accurate conversions.

Proof of Concept

  1. A user provides a span of multiple felt252 elements to the try_into() function of SpanFeltTryIntoByteArray.

  2. The function checks the length of the span and determines that it contains more than one element.

  3. The function attempts to deserialize the entire span using Serde::deserialize(ref self).

  4. The deserialization process fails to correctly convert the span into a ByteArray, resulting in an incorrect ByteArray object.

  5. Any subsequent logic relying on the ByteArray object operates on corrupted or incorrect data, leading to potential errors and vulnerabilities.

Tools Used

Manual review

Recommendation

The deserialization logic should be updated to correctly handle the conversion of multiple felt252 elements into a ByteArray. The following code provides a corrected implementation:

impl SpanFeltTryIntoByteArray of TryInto<Span<felt252>, ByteArray> {
fn try_into(self: Span<felt252>) -> Option<ByteArray> {
if self.len() == 0_usize {
Option::None(())
} else if self.len() == 1_usize {
(*self[0]).try_into()
} else {
let mut res: ByteArray = Default::default();
for felt in self.iter() {
let byte_array: ByteArray = match felt.try_into() {
Option::Some(ba) => ba,
Option::None => return Option::None,
};
res.data.extend(byte_array.data);
res.pending_word = byte_array.pending_word;
res.pending_word_len = byte_array.pending_word_len;
}
Option::Some(res)
}
}
}

This fix ensures that each felt252 element in the span is individually converted to a ByteArray, and the resulting data is correctly aggregated into a single ByteArray object.

Updates

Lead Judging Commences

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

Support

FAQs

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