From 69f0e0d4e0a43ac87bf261b44b4d298e931ddc9d Mon Sep 17 00:00:00 2001 From: Rodrigo Batista de Moraes Date: Fri, 4 Sep 2026 12:51:28 -0300 Subject: [PATCH] Fix int64_t overflow in signed varint decoder Problem: fstGetSVarint64() accumulates the varint in an int64_t, so a long enough varint shifts past the width of the type, which is undefined behaviour and is reported by UBSan. Solution: accumulate in a uint64_t, dropping the bits that do not fit. --- src/fstapi.c | 11 ++++++----- .../malformed/chain_table_svarint_overlong.fst | Bin 0 -> 495 bytes tests/meson.build | 1 + tests/reader_malformed_files.c | 5 +++++ 4 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 tests/data/malformed/chain_table_svarint_overlong.fst diff --git a/src/fstapi.c b/src/fstapi.c index 15440f0..7e577e2 100644 --- a/src/fstapi.c +++ b/src/fstapi.c @@ -617,26 +617,27 @@ static int fstWriterVarint(FILE *handle, uint64_t v) static int64_t fstGetSVarint64(unsigned char *mem, int *skiplen) { unsigned char *mem_orig = mem; - int64_t rc = 0; - const int64_t one = 1; + uint64_t rc = 0; const int siz = sizeof(int64_t) * 8; int shift = 0; unsigned char byt; do { byt = *(mem++); - rc |= ((int64_t)(byt & 0x7f)) << shift; + if (shift < siz) { + rc |= ((uint64_t)(byt & 0x7f)) << shift; + } shift += 7; } while (byt & 0x80); if ((shift < siz) && (byt & 0x40)) { - rc |= -(one << shift); /* sign extend */ + rc |= ~(uint64_t)0 << shift; /* sign extend */ } *skiplen = mem - mem_orig; - return (rc); + return ((int64_t)rc); } #ifndef FST_DYNAMIC_ALIAS2_DISABLE diff --git a/tests/data/malformed/chain_table_svarint_overlong.fst b/tests/data/malformed/chain_table_svarint_overlong.fst new file mode 100644 index 0000000000000000000000000000000000000000..29427a6791ae81f8c7534f8dc1c798f90100ada7 GIT binary patch literal 495 zcmZQz00Tx(2n{D0Gs8u?x%RUj2r)4-GFC7~F)*+-Hb8&_jM2~t(<8zF uR}K|og3-)Sh9s1RDP&}1fr@BBX*T(8sAHIg6FgE9QWzNInWk#60i^*tup6!b literal 0 HcmV?d00001 diff --git a/tests/meson.build b/tests/meson.build index a2aa892..a255a75 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -23,6 +23,7 @@ malformed_cases = [ 'chain_shorter_than_its_length', 'chain_table_overrun', 'chain_table_pair_unterminated', + 'chain_table_svarint_overlong', 'chain_table_unterminated', 'chain_uncompresses_past_traversal', 'chain_varint_pair_past_traversal', diff --git a/tests/reader_malformed_files.c b/tests/reader_malformed_files.c index a02ffab..88cb598 100644 --- a/tests/reader_malformed_files.c +++ b/tests/reader_malformed_files.c @@ -33,6 +33,11 @@ * starts runs off the end of the table, reads as the zero that means * "this entry is two varints", and the second varint is not there * + * chain_table_svarint_overlong.fst + * a chain table holding two signed varints: one nine bytes long, ending + * at bit 63 with its sign bit set, and one eleven bytes long, with bit + * 63 set and bits past the 64th + * * chain_table_unterminated.fst * the last byte of a chain table asks for another byte, so a parse that * stops on reaching the end of the table exactly runs past it