Skip to content

Commit 5f2ecbf

Browse files
committed
fix: Add buffer bounds validation to Cython deserializers
Add bounds checking to prevent buffer overruns and properly handle CQL protocol value semantics in deserializers. Changes: - subelem(): Add bounds validation with protocol-compliant value handling * Happy path: Check elemlen >= 0 and offset + elemlen <= buf.size * Support NULL values (elemlen == -1) per CQL protocol * Support "not set" values (elemlen == -2) per CQL protocol * Reject invalid values (elemlen < -2) with clear error message - _unpack_len(): Add bounds check before reading int32 length field * Validates offset + 4 <= buf.size before pointer dereference * Prevents reading beyond buffer boundaries - DesTupleType: Add defensive bounds checking for tuple deserialization * Check p + 4 <= buf.size before reading item length * Check p + itemlen <= buf.size before reading item data * Explicit NULL value handling (itemlen < 0) * Clear error messages for buffer overruns - DesCompositeType: Add bounds validation for composite type elements * Check 2 + element_length + 1 <= buf.size (length + data + EOC byte) * Prevents buffer overrun when reading composite elements - DesVectorType._deserialize_generic(): Add size validation * Verify buf.size == expected_size before processing * Provides clear error message with expected vs actual sizes Protocol specification reference: [value] = [int] n, followed by n bytes if n >= 0 n == -1: NULL value n == -2: not set value n < -2: invalid (error) Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
1 parent 30d5868 commit 5f2ecbf

3 files changed

Lines changed: 56 additions & 54 deletions

File tree

benchmarks/vector_deserialize.py

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@
3232
# Add parent directory to path
3333
sys.path.insert(0, '.')
3434

35-
from cassandra.cqltypes import FloatType, DoubleType, Int32Type, LongType, ShortType
36-
from cassandra.marshal import float_pack, double_pack, int32_pack, int64_pack, int16_pack
35+
from cassandra.cqltypes import FloatType, DoubleType, Int32Type, LongType
36+
from cassandra.marshal import float_pack, double_pack, int32_pack, int64_pack
3737

3838

3939
def create_test_data(vector_size, element_type):
@@ -50,9 +50,6 @@ def create_test_data(vector_size, element_type):
5050
elif element_type == LongType:
5151
values = list(range(vector_size))
5252
pack_fn = int64_pack
53-
elif element_type == ShortType:
54-
values = list(range(min(vector_size, 32767)))
55-
pack_fn = int16_pack
5653
else:
5754
raise ValueError(f"Unsupported element type: {element_type}")
5855

@@ -91,8 +88,6 @@ def benchmark_struct_optimization(vector_type, serialized_data, iterations=10000
9188
format_str = f'>{vector_size}i'
9289
elif subtype is LongType or (isinstance(subtype, type) and issubclass(subtype, LongType)):
9390
format_str = f'>{vector_size}q'
94-
elif subtype is ShortType or (isinstance(subtype, type) and issubclass(subtype, ShortType)):
95-
format_str = f'>{vector_size}h'
9691
else:
9792
return None, None, None
9893

@@ -126,8 +121,6 @@ def benchmark_numpy_optimization(vector_type, serialized_data, iterations=10000)
126121
dtype = '>i4'
127122
elif subtype is LongType or (isinstance(subtype, type) and issubclass(subtype, LongType)):
128123
dtype = '>i8'
129-
elif subtype is ShortType or (isinstance(subtype, type) and issubclass(subtype, ShortType)):
130-
dtype = '>i2'
131124
else:
132125
return None, None, None
133126

cassandra/cqltypes.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,11 +1479,6 @@ def deserialize(cls, byts, protocol_version):
14791479
if use_numpy:
14801480
return np.frombuffer(byts, dtype='>i8', count=cls.vector_size).tolist()
14811481
return list(struct.unpack(f'>{cls.vector_size}q', byts))
1482-
elif cls.subtype is ShortType or (isinstance(cls.subtype, type) and issubclass(cls.subtype, ShortType)):
1483-
if use_numpy:
1484-
return np.frombuffer(byts, dtype='>i2', count=cls.vector_size).tolist()
1485-
return list(struct.unpack(f'>{cls.vector_size}h', byts))
1486-
14871482
# Fallback: element-by-element deserialization for other fixed-size types
14881483
result = [None] * cls.vector_size
14891484
subtype_deserialize = cls.subtype.deserialize

cassandra/deserializers.pyx

Lines changed: 54 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -203,9 +203,6 @@ cdef inline bint _is_int32_type(object subtype):
203203
cdef inline bint _is_int64_type(object subtype):
204204
return subtype is cqltypes.LongType or issubclass(subtype, cqltypes.LongType)
205205

206-
cdef inline bint _is_int16_type(object subtype):
207-
return subtype is cqltypes.ShortType or issubclass(subtype, cqltypes.ShortType)
208-
209206
cdef inline list _deserialize_numpy_vector(Buffer *buf, int vector_size, str dtype):
210207
"""Unified numpy deserialization for large vectors"""
211208
return np.frombuffer(buf.ptr[:buf.size], dtype=dtype, count=vector_size).tolist()
@@ -279,16 +276,6 @@ cdef class DesVectorType(Deserializer):
279276
raise ValueError(
280277
f"Expected vector of type {self.subtype.typename} and dimension {self.vector_size} "
281278
f"to have serialized size {expected_size}; observed serialized size of {buf.size} instead")
282-
elif _is_int16_type(self.subtype):
283-
elem_size = 2
284-
expected_size = self.vector_size * elem_size
285-
if buf.size == expected_size:
286-
if use_numpy:
287-
return _deserialize_numpy_vector(buf, self.vector_size, '>i2')
288-
return self._deserialize_int16(buf)
289-
raise ValueError(
290-
f"Expected vector of type {self.subtype.typename} and dimension {self.vector_size} "
291-
f"to have serialized size {expected_size}; observed serialized size of {buf.size} instead")
292279
else:
293280
# Unsupported type, use generic deserialization
294281
return self._deserialize_generic(buf, protocol_version)
@@ -372,19 +359,6 @@ cdef class DesVectorType(Deserializer):
372359

373360
return result
374361

375-
cdef inline list _deserialize_int16(self, Buffer *buf):
376-
"""Deserialize int16/short vector using direct C-level access with ntohs"""
377-
cdef Py_ssize_t i
378-
cdef list result
379-
cdef int16_t temp
380-
381-
result = [None] * self.vector_size
382-
for i in range(self.vector_size):
383-
temp = <int16_t>ntohs((<uint16_t*>(buf.ptr + i * 2))[0])
384-
result[i] = temp
385-
386-
return result
387-
388362
cdef inline list _deserialize_generic(self, Buffer *buf, int protocol_version):
389363
"""Fallback: element-by-element deserialization for non-optimized types"""
390364
cdef Py_ssize_t i
@@ -398,6 +372,13 @@ cdef class DesVectorType(Deserializer):
398372
f"VectorType with variable-size subtype {self.subtype.typename} "
399373
"is not supported in Cython deserializer")
400374

375+
# Validate total size before processing
376+
cdef int expected_size = self.vector_size * serialized_size
377+
if buf.size != expected_size:
378+
raise ValueError(
379+
f"Expected vector of type {self.subtype.typename} and dimension {self.vector_size} "
380+
f"to have serialized size {expected_size}; observed serialized size of {buf.size} instead")
381+
401382
for i in range(self.vector_size):
402383
from_ptr_and_size(buf.ptr + offset, serialized_size, &elem_buf)
403384
result[i] = self.subtype.deserialize(to_bytes(&elem_buf), protocol_version)
@@ -473,18 +454,37 @@ cdef inline int subelem(
473454
Read the next element from the buffer: first read the size (in bytes) of the
474455
element, then fill elem_buf with a newly sliced buffer of this size (and the
475456
right offset).
457+
458+
Protocol: n >= 0: n bytes follow
459+
n == -1: NULL value
460+
n == -2: not set value
461+
n < -2: invalid
476462
"""
477463
cdef int32_t elemlen
478464

479465
_unpack_len(buf, offset[0], &elemlen)
480466
offset[0] += sizeof(int32_t)
481-
from_ptr_and_size(buf.ptr + offset[0], elemlen, elem_buf)
482-
offset[0] += elemlen
483-
return 0
467+
468+
# Happy path: non-negative length element that fits in buffer
469+
if elemlen >= 0:
470+
if offset[0] + elemlen <= buf.size:
471+
from_ptr_and_size(buf.ptr + offset[0], elemlen, elem_buf)
472+
offset[0] += elemlen
473+
return 0
474+
raise IndexError("Element length %d at offset %d exceeds buffer size %d" % (elemlen, offset[0], buf.size))
475+
# NULL value (-1) or not set value (-2)
476+
elif elemlen == -1 or elemlen == -2:
477+
from_ptr_and_size(NULL, elemlen, elem_buf)
478+
return 0
479+
# Invalid value (n < -2)
480+
else:
481+
raise ValueError("Invalid element length %d at offset %d" % (elemlen, offset[0]))
484482

485483

486484
cdef inline int _unpack_len(Buffer *buf, int offset, int32_t *output) except -1:
487485
"""Read a big-endian int32 at the given offset using direct pointer access."""
486+
if offset + sizeof(int32_t) > buf.size:
487+
raise IndexError("Cannot read length field: offset %d + 4 exceeds buffer size %d" % (offset, buf.size))
488488
cdef uint32_t *src = <uint32_t*>(buf.ptr + offset)
489489
output[0] = <int32_t>ntohl(src[0])
490490
return 0
@@ -556,16 +556,24 @@ cdef class DesTupleType(_DesParameterizedType):
556556
values = []
557557
for i in range(self.subtypes_len):
558558
item = None
559-
if p < buf.size:
559+
if p + 4 <= buf.size:
560560
# Read itemlen directly using ntohl instead of slice_buffer
561561
itemlen = <int32_t>ntohl((<uint32_t*>(buf.ptr + p))[0])
562562
p += 4
563-
if itemlen >= 0:
563+
564+
if itemlen >= 0 and p + itemlen <= buf.size:
564565
from_ptr_and_size(buf.ptr + p, itemlen, &item_buf)
565566
p += itemlen
566567

567568
deserializer = self.deserializers[i]
568569
item = from_binary(deserializer, &item_buf, protocol_version)
570+
elif itemlen < 0:
571+
# NULL value, item stays None
572+
pass
573+
else:
574+
raise IndexError("Tuple item length %d at offset %d exceeds buffer size %d" % (itemlen, p, buf.size))
575+
elif p < buf.size:
576+
raise IndexError("Cannot read tuple item length at offset %d: only %d bytes remain" % (p, buf.size - p))
569577

570578
tuple_set(res, i, item)
571579

@@ -607,17 +615,23 @@ cdef class DesCompositeType(_DesParameterizedType):
607615
break
608616

609617
element_length = unpack_num[uint16_t](buf)
610-
from_ptr_and_size(buf.ptr + 2, element_length, &elem_buf)
611618

612-
deserializer = self.deserializers[i]
613-
item = from_binary(deserializer, &elem_buf, protocol_version)
614-
tuple_set(res, i, item)
619+
# Validate that we have enough data for the element and EOC byte (happy path check)
620+
if 2 + element_length + 1 <= buf.size:
621+
from_ptr_and_size(buf.ptr + 2, element_length, &elem_buf)
622+
623+
deserializer = self.deserializers[i]
624+
item = from_binary(deserializer, &elem_buf, protocol_version)
625+
tuple_set(res, i, item)
615626

616-
# skip element length, element, and the EOC (one byte)
617-
# Advance buffer in-place with direct assignment
618-
start = 2 + element_length + 1
619-
buf.ptr = buf.ptr + start
620-
buf.size = buf.size - start
627+
# skip element length, element, and the EOC (one byte)
628+
# Advance buffer in-place with direct assignment
629+
start = 2 + element_length + 1
630+
buf.ptr = buf.ptr + start
631+
buf.size = buf.size - start
632+
else:
633+
raise IndexError("Composite element length %d requires %d bytes but only %d remain" %
634+
(element_length, 2 + element_length + 1, buf.size))
621635

622636
return res
623637

0 commit comments

Comments
 (0)