CVE-2021-3889: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: libmobi is vulnerable to Use of Out-of-range Pointer Offset
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 8.1
Published: 2021-10-19T13:15Z
Repository: https://github.com/bfabiszewski/libmobi
Language: C
Files changed: src/compression.c, src/compression.h, src/read.c
Commit date: 2021-09-18 22:14:58 +0200
Commit message: Fix potential out-of-buffer read while parsing corrupt file, closes #35, #36

Diff: diff --git a/src/compression.c b/src/compression.c
index 48ac305..cf39232 100644
--- a/src/compression.c
+++ b/src/compression.c
@@ -139,11 +139,14 @@ static MOBI_RET mobi_decompress_huffman_internal(MOBIBuffer *buf_out, MOBIBuffer
         uint32_t maxcode = (((t1 >> 8) + 1) << (32 - code_length)) - 1;
         /* check termination bit */
         if (!(t1 & 0x80)) {
             /* get offset from mincode, maxcode tables */
             while (code < huffcdic->mincode_table[code_length]) {
-                code_length++;
+                if (++code_length >= HUFF_CODETABLE_SIZE) {
+                    debug_print("Wrong offset to mincode table: %hhu\n", code_length);
+                    return MOBI_DATA_CORRUPT;
+                }
             }
             maxcode = huffcdic->maxcode_table[code_length];
         }
         bitcount -= code_length;
         bitsleft -= code_length;
diff --git a/src/compression.h b/src/compression.h
index e18b3c7..59e116e 100644
--- a/src/compression.h
+++ b/src/compression.h
@@ -18,22 +18,23 @@
 #define MOBI_INLINE /**< Syntax for compiler inline keyword from config.h */
 #endif
 
 /* FIXME: what is the reasonable value? */
 #define MOBI_HUFFMAN_MAXDEPTH 20 /**< Maximal recursion level for huffman decompression routine */
+#define HUFF_CODETABLE_SIZE 33 /**< Size of min- and maxcode tables */
 
 
 /**
  @brief Parsed data from HUFF and CDIC records needed to unpack huffman compressed text
  */
 typedef struct {
     size_t index_count; /**< Total number of indices in all CDIC records, stored in each CDIC record header */
     size_t index_read; /**< Number of indices parsed, used by parser */
     size_t code_length; /**< Code length value stored in CDIC record header */
     uint32_t table1[256]; /**< Table of big-endian indices from HUFF record data1 */
-    uint32_t mincode_table[33]; /**< Table of big-endian mincodes from HUFF record data2 */
-    uint32_t maxcode_table[33]; /**< Table of big-endian maxcodes from HUFF record data2 */
+    uint32_t mincode_table[HUFF_CODETABLE_SIZE]; /**< Table of big-endian mincodes from HUFF record data2 */
+    uint32_t maxcode_table[HUFF_CODETABLE_SIZE]; /**< Table of big-endian maxcodes from HUFF record data2 */
     uint16_t *symbol_offsets; /**< Index of symbol offsets parsed from CDIC records (index_count entries) */
     unsigned char **symbols; /**< Array of pointers to start of symbols data in each CDIC record (index = number of CDIC record) */
 } MOBIHuffCdic;
 
 MOBI_RET mobi_decompress_lz77(unsigned char *out, const unsigned char *in, size_t *len_out, const size_t len_in);
diff --git a/src/read.c b/src/read.c
index b90fcf3..0d2d9e4 100644
--- a/src/read.c
+++ b/src/read.c
@@ -587,11 +587,11 @@ MOBI_RET mobi_parse_huff(MOBIHuffCdic *huffcdic, const MOBIPdbRecord *record) {
         return MOBI_DATA_CORRUPT;
     }
     /* read 32 mincode-maxcode pairs from data2 big-endian */
     huffcdic->mincode_table[0] = 0;
     huffcdic->maxcode_table[0] = 0xFFFFFFFF;
-    for (int i = 1; i < 33; i++) {
+    for (int i = 1; i < HUFF_CODETABLE_SIZE; i++) {
         const uint32_t mincode = mobi_buffer_get32(buf);
         const uint32_t maxcode = mobi_buffer_get32(buf);
         huffcdic->mincode_table[i] =  mincode << (32 - i);
         huffcdic->maxcode_table[i] =  ((maxcode + 1) << (32 - i)) - 1;
     }

Vulnerable code:                 code_length++;
    uint32_t mincode_table[33]; /**< Table of big-endian mincodes from HUFF record data2 */
    uint32_t maxcode_table[33]; /**< Table of big-endian maxcodes from HUFF record data2 */
    for (int i = 1; i < 33; i++) {
Fixed code:                 if (++code_length >= HUFF_CODETABLE_SIZE) {
                    debug_print("Wrong offset to mincode table: %hhu\n", code_length);
                    return MOBI_DATA_CORRUPT;
                }
#define HUFF_CODETABLE_SIZE 33 /**< Size of min- and maxcode tables */
    uint32_t mincode_table[HUFF_CODETABLE_SIZE]; /**< Table of big-endian mincodes from HUFF record data2 */
    uint32_t maxcode_table[HUFF_CODETABLE_SIZE]; /**< Table of big-endian maxcodes from HUFF record data2 */
    for (int i = 1; i < HUFF_CODETABLE_SIZE; i++) {