Skip to content

Commit 434e9e7

Browse files
committed
Fix DoS from malicious Thrift container counts in fuzzer slow units
1 parent e782747 commit 434e9e7

File tree

2 files changed

+27
-1
lines changed

2 files changed

+27
-1
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ Carquet is **not** a replacement for Apache Arrow. Arrow is the industry standar
3333
| Write speed (ARM) | Baseline | **1.5-5x faster** |
3434
| Write speed (x86) | Baseline | **1.2-1.4x faster** |
3535
| Read speed (ARM) | Baseline | ~same to 1.3x faster |
36-
| Read speed (x86) | **Baseline** | 1.5-2x slower |
36+
| Read speed (x86) | Baseline | 1.5-2x slower |
3737
| ZSTD file size | Baseline | **~1.4x smaller** |
3838
| Nested types | **Full support** | Basic |
3939
| Encryption | **Yes** | No |

src/thrift/thrift_decode.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,16 @@ void thrift_read_list_begin(thrift_decoder_t* dec,
303303
if (*count < 0) {
304304
set_error(dec, CARQUET_ERROR_THRIFT_DECODE, "Negative list size");
305305
*count = 0;
306+
return;
307+
}
308+
309+
/* Each list element consumes at least 1 byte, so count cannot exceed
310+
* remaining data. This prevents billion-iteration busy loops from
311+
* malicious varints in tiny payloads. */
312+
size_t remaining = carquet_buffer_reader_remaining(&dec->reader);
313+
if ((size_t)*count > remaining) {
314+
set_error(dec, CARQUET_ERROR_THRIFT_DECODE, "List count exceeds remaining data");
315+
*count = 0;
306316
}
307317
}
308318

@@ -334,6 +344,18 @@ void thrift_read_map_begin(thrift_decoder_t* dec,
334344
return;
335345
}
336346

347+
/* Each map entry consumes at least 1 byte (types byte already read
348+
* separately), so count cannot exceed remaining data. This prevents
349+
* billion-iteration busy loops from malicious varints. */
350+
size_t remaining = carquet_buffer_reader_remaining(&dec->reader);
351+
if ((size_t)*count > remaining) {
352+
set_error(dec, CARQUET_ERROR_THRIFT_DECODE, "Map count exceeds remaining data");
353+
*count = 0;
354+
*key_type = THRIFT_TYPE_STOP;
355+
*value_type = THRIFT_TYPE_STOP;
356+
return;
357+
}
358+
337359
/* Key and value types in one byte */
338360
uint8_t types = read_byte_raw(dec);
339361
*key_type = (thrift_type_t)((types >> 4) & 0x0F);
@@ -352,6 +374,10 @@ void thrift_skip(thrift_decoder_t* dec, thrift_type_t type) {
352374

353375
switch (type) {
354376
case THRIFT_TYPE_STOP:
377+
/* STOP is a struct terminator, never a value type to skip.
378+
* Treating it as a no-op would cause infinite loops when it
379+
* appears as a container element type from malformed data. */
380+
set_error(dec, CARQUET_ERROR_THRIFT_DECODE, "Cannot skip STOP type");
355381
break;
356382

357383
case THRIFT_TYPE_TRUE:

0 commit comments

Comments
 (0)