IMPORT: slz/uslz: widen crc_flush, it overflowed on large stored blocks

crc_flush counts the bytes decoded but not checksummed yet. It was
declared as a (signed) short, while a single stored (uncompressed) block
copy can add up to a whole output buffer in one call, making it overflow:

    crc_flush=-32768 after +=32768 (index=32768)

That happens with any incompressible payload, which the encoders emit as
stored blocks, as soon as the input is fed in chunks large enough for the
copy to advance by more than 32767 bytes at once. The subsequent call to
uslz_update_crc() then gets a negative length. The checksum functions
happen not to touch memory in that case (their loops simply do not run),
so the visible symptom is a wrong checksum and a valid stream rejected
with USLZ_DECODE_E_BAD_CRC -- but the invariant crc_flush <= index is
broken from then on, and (out_base + index - crc_flush) can point before
the output buffer, which would read out of bounds.

Here we perform two changes:

  - crc_flush becomes an int. It has to be larger than the output
    buffer, not smaller.

  - the CRC_BLOCK batch flush becomes a while loop instead of an if.
    It only ever flushed one CRC_BLOCK per emitted chunk, so a large
    copy left the rest pending and defeated the point of batching the
    checksum in blocks small enough to stay in L1. With the loop,
    crc_flush is back below CRC_BLOCK after every emission, which also
    bounds it tightly.

It's easy to reproduce with a 40kB incompressible file (e.g. urandom):

    gzip -9 -c < rand40k > r.gz9
    ./uslztest 32768 1000000 < r.gz9     # was: error (5), now: crc=195cf5ee

tests/uslztest.sh goes from 1488/1514 to 1510/1514. The four remaining
failures are for future patches.

This is libslz upstream commit 96b80bb978e676db75a631bf63aaf5d32c22f015

diff --git a/include/import/slz.h b/include/import/slz.h
index 5eabe4ef2bf7834323a4e5d7371c3d65d955d3e7..69c74747d22ff2e0c8fa95f88b97515941cb33b3 100644 (file)
--- a/include/import/slz.h
+++ b/include/import/slz.h
@@ -233,8 +233,11 @@ struct uslz_stream {
        enum uslz_stream_state state;         /* parsing state, USLZ_ST_* */
 
        uint32_t crc;                         /* current crc value for the stream */
-       short crc_flush;                      /* byte counter to know when to perform
-                                              * the next crc computing batch
+       int crc_flush;                        /* number of bytes decoded but not
+                                              * checksummed yet. Must be wider than
+                                              * the output buffer since a single
+                                              * stored block copy can add up to a
+                                              * full buffer at once.
                                               */
        uint16_t flags;                       /* USLZ_FL_* flags */
        unsigned int counter;                 /* generic counter */

diff --git a/src/uslz.c b/src/uslz.c
index ef744ca23b119bc801b26ce88808042d23c69651..e5d85842a941c3588358e3fcf575dbeeb63f64ec 100644 (file)
--- a/src/uslz.c
+++ b/src/uslz.c
@@ -397,7 +397,7 @@ static inline uint8_t rbit5(uint8_t v)
                        state->crc_flush = 0;                        \
                        index = 0;                                   \
                }                                                    \
-               if (state->crc_flush >= CRC_BLOCK) {                 \
+               while (state->crc_flush >= CRC_BLOCK) {              \
                        uslz_update_crc(state, out_base + index - state->crc_flush, CRC_BLOCK); \
                        state->crc_flush -= CRC_BLOCK;               \
                }                                                    \