IMPORT: slz/uslz: verify the trailer checksum before reporting completion

uslz_decode_block() sets USLZ_FL_COMPLETE at the top of its completion
path, before reading and comparing the trailer checksum. If the trailer
bytes were not available yet, we jump to out_of_data, and uslz_decode()
then sees a complete stream in its drain path and jumps to end,
returning USLZ_DECODE_SUCCESS without ever calling uslz_decode_block()
again. Thus the checksum is never verified at all in this case, meaning
that under some scheduling circumstances, an invalid stream could appear
as valid.

On top of that, nothing ever assigns USLZ_ST_CHECK_CKSUM, so even if the
decoder had been called again, the state machine could not have resumed
in the middle of the trailer: it would have gone back to decoding symbols
and interpreted the trailer as deflate data.

The result was that a stream with a corrupted checksum was accepted,
silently, depending only on where the caller happened to cut its input.
On a 319-byte zlib stream the corruption went unnoticed at chunk sizes
1 to 7, 9, 15, 21, 35, 45 and 53, and on a 331-byte gzip stream with a
corrupted crc32 at 12, 13, 17, 18, 19, 25, 27, 36 and 54. (reminder,
deflate has no checksum).

This patch addresses this problem this way:

  - USLZ_ST_CHECK_CKSUM is set before reading the trailer, so that an
    out of data return really can resume there.

  - crc_flush is cleared after the final uslz_update_crc(), so as not
    to checksum the same bytes a second time in case of resume.

  - USLZ_FL_COMPLETE is set only *after* the comparison succeeded.

Now, trying to decode a corrupted trailer for either a zlib or gzip
stream properly reports a corruption (tested with 80 positions for each).

This is libslz upstream commit 1dc3cd773637477e5de33609d63527a0fd9bc4e7

diff --git a/src/uslz.c b/src/uslz.c
index e5d85842a941c3588358e3fcf575dbeeb63f64ec..d995440c1b15e249a5155ed32fa3577f24cdd94f 100644 (file)
--- a/src/uslz.c
+++ b/src/uslz.c
@@ -857,16 +857,23 @@ static enum uslz_decode_ret uslz_decode_block(struct uslz_stream *state)
 
        /* only on the very last return */
        if ((state->flags & USLZ_FL_FINAL)) {
-               state->flags |= USLZ_FL_COMPLETE;
                /* checksum checks for relevant formats: we use the bit
                 * accumulator for that purpose since it is not used for
                 * decoding anymore.
                 */
                bit_accum = 0;
                num_bits = 0;
+               /* the trailer may well not be available yet, in which case we
+                * have to be able to come back here on the next call.
+                */
+               state->state = USLZ_ST_CHECK_CKSUM;
 
  state_CHECK_CKSUM:
+               /* checksum what has not been checksummed yet, and clear the
+                * counter so that resuming here does not do it twice.
+                */
                uslz_update_crc(state, out_base + index - state->crc_flush, state->crc_flush);
+               state->crc_flush = 0;
 
                if ((state->flags & USLZ_FL_ZLIB)) {
                        /* check computed zlib adler checksum against 4 last bytes
@@ -901,6 +908,15 @@ static enum uslz_decode_ret uslz_decode_block(struct uslz_stream *state)
                                goto error_return;
                        }
                }
+
+               /* The trailer has been verified, and only now may the stream
+                * be reported as complete. Doing it before the check meant
+                * that if the trailer was not available yet, the out of data
+                * return above would make uslz_decode() see a complete stream
+                * and return success from its drain path without ever coming
+                * back here, so the checksum was silently never verified.
+                */
+               state->flags |= USLZ_FL_COMPLETE;
        }
 
        state->in_ptr    = in_ptr;