Fixing crashes with malformed compressed data

The Gorilla and Decitionary compressor crashed on malformed data
with the reverse decompression iterator and the bulk decompressor.

This change adds validation for these cases and adds reverse iterator
coverage to the fuzzer tests, so the randomised test will catch future issues.

This issue was reported by Mehmet Ince @mdisec https://mehmetince.net/, and
the test cases are his contribution with tiny modifications to fit them into
the test suite.

Author: dbeck

.unreleased/pr_10360

@@ -0,0 +1,2 @@
+Fixes: #10360 Decompressor crashes with malformed compressed data
+Thanks: @mdisec for reporting the issues with the validation of the compressed data during decompression

src/adts/bit_array.h

@@ -37,6 +37,7 @@ pg_attribute_always_inline static uint64 bit_array_iter_next(BitArrayIterator *i
 static void bit_array_iterator_init_rev(BitArrayIterator *iter, const BitArray *array);
 /* return last num_bits in forward-order (not reverse-order); must have been written as num_bits */
 static uint64 bit_array_iter_next_rev(BitArrayIterator *iter, uint8 num_bits);
+static inline uint64 bit_array_iter_position(const BitArrayIterator *iter);
 
 /* I/O */
 static inline void bit_array_send(StringInfo buffer, const BitArray *data);

src/adts/bit_array_impl.h

@@ -250,6 +250,12 @@ bit_array_iterator_init(BitArrayIterator *iter, const BitArray *array)
 	};
 }
 
+static inline uint64
+bit_array_iter_position(const BitArrayIterator *iter)
+{
+	return (BITS_PER_BUCKET * iter->current_bucket) + iter->bits_used_in_current_bucket;
+}
+
 pg_attribute_always_inline static uint64
 bit_array_iter_next(BitArrayIterator *iter, uint8 num_bits)
 {

tsl/src/compression/algorithms/deltadelta.c

@@ -481,6 +481,44 @@ delta_delta_compressor_append_value(DeltaDeltaCompressor *compressor, int64 next
 /**********************************************************************************/
 /**********************************************************************************/
 
+static inline uint32
+decompression_iterator_items_seen(const DeltaDeltaDecompressionIterator *iter)
+{
+	if (iter->has_nulls)
+	{
+		return iter->nulls.num_elements_returned;
+	}
+	else
+	{
+		return iter->delta_deltas.num_elements_returned;
+	}
+}
+
+static inline uint32
+decompression_iterator_item_count(const DeltaDeltaDecompressionIterator *iter)
+{
+	if (iter->has_nulls)
+	{
+		return iter->nulls.num_elements;
+	}
+	else
+	{
+		return iter->delta_deltas.num_elements;
+	}
+}
+
+static inline uint32
+decompression_iterator_values_seen(const DeltaDeltaDecompressionIterator *iter)
+{
+	return iter->delta_deltas.num_elements_returned;
+}
+
+static inline uint32
+decompression_iterator_value_count(const DeltaDeltaDecompressionIterator *iter)
+{
+	return iter->delta_deltas.num_elements;
+}
+
 static void
 int64_decompression_iterator_init_forward(DeltaDeltaDecompressionIterator *iter, void *compressed,
 										  Oid element_type)
@@ -515,6 +553,7 @@ int64_decompression_iterator_init_forward(DeltaDeltaDecompressionIterator *iter,
 	{
 		Simple8bRleSerialized *nulls = bytes_deserialize_simple8b_and_advance(&si);
 		simple8brle_decompression_iterator_init_forward(&iter->nulls, nulls);
+		CheckCompressedData(deltas->num_elements <= nulls->num_elements);
 	}
 }
 
@@ -529,7 +568,7 @@ int64_decompression_iterator_init_reverse(DeltaDeltaDecompressionIterator *iter,
 	DeltaDeltaCompressed *header = consumeCompressedData(&si, sizeof(DeltaDeltaCompressed));
 	Simple8bRleSerialized *deltas = bytes_deserialize_simple8b_and_advance(&si);
 
-	Assert(header->has_nulls == 0 || header->has_nulls == 1);
+	CheckCompressedData(header->has_nulls == 0 || header->has_nulls == 1);
 
 	*iter = (DeltaDeltaDecompressionIterator){
 		.base = {
@@ -549,6 +588,7 @@ int64_decompression_iterator_init_reverse(DeltaDeltaDecompressionIterator *iter,
 	{
 		Simple8bRleSerialized *nulls = bytes_deserialize_simple8b_and_advance(&si);
 		simple8brle_decompression_iterator_init_reverse(&iter->nulls, nulls);
+		CheckCompressedData(deltas->num_elements <= nulls->num_elements);
 	}
 }
 
@@ -615,6 +655,12 @@ delta_delta_decompression_iterator_try_next_forward_internal(DeltaDeltaDecompres
 			simple8brle_decompression_iterator_try_next_forward(&iter->nulls);
 		if (result.is_done)
 		{
+			/* make sure we exhausted all items before  */
+			CheckCompressedData(decompression_iterator_items_seen(iter) ==
+								decompression_iterator_item_count(iter));
+			/* and also that we returned all values */
+			CheckCompressedData(decompression_iterator_values_seen(iter) ==
+								decompression_iterator_value_count(iter));
 			return (DecompressResultInternal){
 				.is_done = true,
 			};
@@ -633,6 +679,9 @@ delta_delta_decompression_iterator_try_next_forward_internal(DeltaDeltaDecompres
 
 	if (result.is_done)
 	{
+		/* make sure we exhausted all items before  */
+		CheckCompressedData(decompression_iterator_items_seen(iter) ==
+							decompression_iterator_item_count(iter));
 		return (DecompressResultInternal){
 			.is_done = true,
 		};
@@ -712,14 +761,20 @@ delta_delta_decompression_iterator_try_next_reverse_internal(DeltaDeltaDecompres
 			simple8brle_decompression_iterator_try_next_reverse(&iter->nulls);
 		if (result.is_done)
 		{
+			/* make sure we exhausted all items before  */
+			CheckCompressedData(decompression_iterator_items_seen(iter) ==
+								decompression_iterator_item_count(iter));
+			/* and also that we returned all values */
+			CheckCompressedData(decompression_iterator_values_seen(iter) ==
+								decompression_iterator_value_count(iter));
 			return (DecompressResultInternal){
 				.is_done = true,
 			};
 		}
 
 		if (result.val != 0)
 		{
-			Assert(result.val == 1);
+			CheckCompressedData(result.val == 1);
 			return (DecompressResultInternal){
 				.is_null = true,
 			};
@@ -730,6 +785,9 @@ delta_delta_decompression_iterator_try_next_reverse_internal(DeltaDeltaDecompres
 
 	if (result.is_done)
 	{
+		/* make sure we exhausted all items before  */
+		CheckCompressedData(decompression_iterator_items_seen(iter) ==
+							decompression_iterator_item_count(iter));
 		return (DecompressResultInternal){
 			.is_done = true,
 		};

tsl/src/compression/algorithms/dictionary.c

@@ -719,7 +719,8 @@ tsl_text_dictionary_decompress_all(Datum compressed, Oid element_type, MemoryCon
 	bool have_incorrect_index = false;
 	for (uint32 i = 0; i < n_notnull; i++)
 	{
-		have_incorrect_index = have_incorrect_index || indices[i] >= (int16) header->num_distinct;
+		have_incorrect_index =
+			have_incorrect_index || indices[i] >= (int16) header->num_distinct || indices[i] < 0;
 	}
 	CheckCompressedData(!have_incorrect_index);
 
@@ -897,7 +898,7 @@ dictionary_decompression_iterator_try_next_reverse(DecompressionIterator *iter_b
 		};
 	}
 
-	Assert(result.val < iter->compressed->num_distinct);
+	CheckCompressedData(result.val < iter->compressed->num_distinct);
 	return (DecompressResult){
 		.val = iter->values[result.val],
 		.is_null = false,

tsl/src/compression/algorithms/gorilla.c

@@ -796,10 +796,15 @@ gorilla_decompression_iterator_from_datum_reverse(Datum gorilla_compressed, Oid
 	}
 
 	/* we need to know how many bits are used, even if the last value didn't store them */
+	int num_leading_zeros_bits = bit_array_num_bits(&iter->gorilla_data.leading_zeros);
+
+	/* leading zeros have a fixed size and we need at least one item */
+	CheckCompressedData(num_leading_zeros_bits >= BITS_PER_LEADING_ZEROS &&
+						num_leading_zeros_bits % BITS_PER_LEADING_ZEROS == 0);
 	iter->prev_leading_zeroes =
 		bit_array_iter_next_rev(&iter->leading_zeros, BITS_PER_LEADING_ZEROS);
 	num_xor_bits = simple8brle_decompression_iterator_try_next_reverse(&iter->num_bits_used);
-	Assert(!num_xor_bits.is_done);
+	CheckCompressedData(!num_xor_bits.is_done);
 	iter->prev_xor_bits_used = num_xor_bits.val;
 	iter->prev_val = iter->gorilla_data.header->last_value;
 	return &iter->base;
@@ -851,6 +856,10 @@ gorilla_decompression_iterator_try_next_reverse_internal(GorillaDecompressionIte
 		};
 	}
 
+	/* make sure we are in a valid state */
+	tag1 = simple8brle_decompression_iterator_try_next_reverse(&iter->tag1s);
+	CheckCompressedData(!tag1.is_done);
+
 	xor = bit_array_iter_next_rev(&iter->xors, iter->prev_xor_bits_used);
 
 	if (iter->prev_leading_zeroes + iter->prev_xor_bits_used < 64)
@@ -859,8 +868,6 @@ gorilla_decompression_iterator_try_next_reverse_internal(GorillaDecompressionIte
 	}
 	iter->prev_val ^= xor;
 
-	tag1 = simple8brle_decompression_iterator_try_next_reverse(&iter->tag1s);
-
 	if (tag1.val != 0)
 	{
 		/* get new xor sizes */
@@ -875,6 +882,9 @@ gorilla_decompression_iterator_try_next_reverse_internal(GorillaDecompressionIte
 		}
 		else
 		{
+			/* make sure we have enough bits to read for the leading zeros */
+			CheckCompressedData(bit_array_iter_position(&iter->leading_zeros) >=
+								BITS_PER_LEADING_ZEROS);
 			iter->prev_xor_bits_used = num_xor_bits.val;
 			iter->prev_leading_zeroes =
 				bit_array_iter_next_rev(&iter->leading_zeros, BITS_PER_LEADING_ZEROS);

tsl/src/compression/algorithms/simple8b_rle.h

@@ -6,6 +6,7 @@
 #pragma once
 
 #include <postgres.h>
+#include "compression/compression.h"
 #include <c.h>
 #include <fmgr.h>
 #include <lib/stringinfo.h>
@@ -1007,12 +1008,12 @@ simple8brle_decompression_iterator_max_elements(Simple8bRleDecompressionIterator
 
 		if (simple8brle_selector_is_rle(selector) && iter->compressed_data)
 		{
-			Assert(simple8brle_rledata_repeatcount(iter->compressed_data[i]) > 0);
+			CheckCompressedData(simple8brle_rledata_repeatcount(iter->compressed_data[i]) > 0);
 			max_stored += simple8brle_rledata_repeatcount(iter->compressed_data[i]);
 		}
 		else
 		{
-			Assert(selector < SIMPLE8B_MAXCODE);
+			CheckCompressedData(selector < SIMPLE8B_MAXCODE);
 			max_stored += SIMPLE8B_NUM_ELEMENTS[selector];
 		}
 	}
@@ -1029,6 +1030,7 @@ simple8brle_decompression_iterator_init_reverse(Simple8bRleDecompressionIterator
 	skipped_in_last = simple8brle_decompression_iterator_max_elements(iter, compressed) -
 					  compressed->num_elements;
 
+	CheckCompressedData(skipped_in_last >= 0);
 	Assert(NULL != iter->compressed_data);
 
 	iter->current_block =
@@ -1098,6 +1100,9 @@ simple8brle_decompression_iterator_try_next_reverse(Simple8bRleDecompressionIter
 			simple8brle_block_create(bit_array_iter_next_rev(&iter->selectors,
 															 SIMPLE8B_BITS_PER_SELECTOR),
 									 iter->compressed_data[iter->current_compressed_pos]);
+		CheckCompressedData(iter->current_block.selector != 0);
+		CheckCompressedData(iter->current_block.num_elements_compressed <=
+							GLOBAL_MAX_ROWS_PER_COMPRESSION);
 		iter->current_in_compressed_pos = iter->current_block.num_elements_compressed - 1;
 		iter->current_compressed_pos -= 1;
 	}

tsl/src/compression/algorithms/uuid_compress.c

@@ -662,12 +662,15 @@ uuid_decompress_all(Datum compressed, Oid element_type, MemoryContext dest_mctx)
 		{
 			if (arrow_row_is_valid(validity_bitmap, i))
 			{
-				Assert(value_position < num_values);
+				CheckCompressedData(value_position < num_values);
 				uuid_buffer[i].components[0] = pg_ntoh64(timestamp_values[i]);
 				uuid_buffer[i].components[1] = rand_b_and_variant[value_position];
 				++value_position;
 			}
 		}
+
+		/* check that we returned all values */
+		CheckCompressedData(value_position == num_values);
 	}
 	else
 	{

tsl/test/expected/compression_algos.out

@@ -2151,10 +2151,11 @@ group by 2, 3 order by 1 desc
 ;
  count | bulk_result | rowbyrow_result 
 -------+-------------+-----------------
-   144 | XX001       | true
-    93 | XX001       | XX001
-    54 | true        | true
+   221 | XX001       | XX001
+    43 | XX000       | XX000
+    26 | XX001       | XX000
     24 | 08P01       | 08P01
+     1 | true        | true
 
 \set algo deltadelta
 \set type int8
@@ -2166,9 +2167,9 @@ group by 2, 3 order by 1 desc
 ;
  count | bulk_result | rowbyrow_result 
 -------+-------------+-----------------
-   108 | XX001       | XX001
-    69 | true        | true
-    68 | XX001       | true
+   143 | XX001       | XX001
+    67 | XX000       | XX000
+    35 | XX001       | XX000
     13 | 08P01       | 08P01
      1 | false       | false
 
@@ -2218,8 +2219,9 @@ group by 2, 3 order by 1 desc
  count | bulk_result | rowbyrow_result 
 -------+-------------+-----------------
     18 | XX001       | XX001
-     3 | true        | true
      2 | 08P01       | 08P01
+     2 | XX000       | XX000
+     1 | true        | true
 
 \set algo uuid
 \set type uuid
@@ -2233,6 +2235,6 @@ group by 2, 3 order by 1 desc
 -------+-------------+-----------------
     49 | XX001       | XX001
      2 | 08P01       | 08P01
-     1 | true        | XX001
+     1 | XX000       | XX000
      1 | true        | true