From 6139a0b9e93fcb7fcf423e56aa825bc869e02229 Mon Sep 17 00:00:00 2001 From: Stuart Caie Date: Sun, 13 Aug 2017 12:54:04 +0100 Subject: [PATCH] Reject negative output length in SpanInfo --- libmspack/ChangeLog | 13 +++++++++++++ libmspack/mspack/chmd.c | 10 ++++++++-- libmspack/mspack/lzxd.c | 10 ++++++++-- libmspack/mspack/mszipd.c | 3 ++- libmspack/mspack/qtmd.c | 1 + 5 files changed, 32 insertions(+), 5 deletions(-) diff --git a/libmspack/ChangeLog b/libmspack/ChangeLog index 90b0b48..f6a39f9 100644 --- a/libmspack/ChangeLog +++ b/libmspack/ChangeLog @@ -1,3 +1,16 @@ +2017-08-13 Stuart Caie + + * read_spaninfo(): a CHM file can have no ResetTable and have a + negative length in SpanInfo, which then feeds a negative output length + to lzxd_init(), which then sets frame_size to a value of your choosing, + the lower 32 bits of output length, larger than LZX_FRAME_SIZE. If the + first LZX block is uncompressed, this writes data beyond the end of the + window. This issue was raised by ClamAV as CVE-2017-6419. Thanks to + Sebastian Andrzej Siewior for finding this by chance! + + * lzxd_init(), lzxd_set_output_length(), mszipd_init(): due to the issue + mentioned above, these functions now reject negative lengths + 2017-08-05 Stuart Caie * cabd_read_string(): add missing error check on result of read(). diff --git a/libmspack/mspack/chmd.c b/libmspack/mspack/chmd.c index 5a6ef54..1a486c8 100644 --- a/libmspack/mspack/chmd.c +++ b/libmspack/mspack/chmd.c @@ -1269,9 +1269,15 @@ static int read_spaninfo(struct mschm_decompressor_p *self, /* get the uncompressed length of the LZX stream */ err = read_off64(length_ptr, data, sys, self->d->infh); - sys->free(data); - return (err) ? MSPACK_ERR_DATAFORMAT : MSPACK_ERR_OK; + if (err) return MSPACK_ERR_DATAFORMAT; + + if (*length_ptr <= 0) { + D(("output length is invalid")) + return MSPACK_ERR_DATAFORMAT; + } + + return MSPACK_ERR_OK; } /*************************************** diff --git a/libmspack/mspack/lzxd.c b/libmspack/mspack/lzxd.c index 2281e7b..d164df9 100644 --- a/libmspack/mspack/lzxd.c +++ b/libmspack/mspack/lzxd.c @@ -300,8 +300,14 @@ struct lzxd_stream *lzxd_init(struct mspack_system *system, if (window_bits < 15 || window_bits > 21) return NULL; } + if (reset_interval < 0 || output_length < 0) { + D(("reset interval or output length < 0")) + return NULL; + } + + /* round up input buffer size to multiple of two */ input_buffer_size = (input_buffer_size + 1) & -2; - if (!input_buffer_size) return NULL; + if (input_buffer_size < 2) return NULL; /* allocate decompression state */ if (!(lzx = (struct lzxd_stream *) system->alloc(system, sizeof(struct lzxd_stream)))) { @@ -382,7 +388,7 @@ int lzxd_set_reference_data(struct lzxd_stream *lzx, } void lzxd_set_output_length(struct lzxd_stream *lzx, off_t out_bytes) { - if (lzx) lzx->length = out_bytes; + if (lzx && out_bytes > 0) lzx->length = out_bytes; } int lzxd_decompress(struct lzxd_stream *lzx, off_t out_bytes) { diff --git a/libmspack/mspack/mszipd.c b/libmspack/mspack/mszipd.c index 5b4756d..6ecd96d 100644 --- a/libmspack/mspack/mszipd.c +++ b/libmspack/mspack/mszipd.c @@ -349,8 +349,9 @@ struct mszipd_stream *mszipd_init(struct mspack_system *system, if (!system) return NULL; + /* round up input buffer size to multiple of two */ input_buffer_size = (input_buffer_size + 1) & -2; - if (!input_buffer_size) return NULL; + if (input_buffer_size < 2) return NULL; /* allocate decompression state */ if (!(zip = (struct mszipd_stream *) system->alloc(system, sizeof(struct mszipd_stream)))) { diff --git a/libmspack/mspack/qtmd.c b/libmspack/mspack/qtmd.c index 12b27f5..5d2c76f 100644 --- a/libmspack/mspack/qtmd.c +++ b/libmspack/mspack/qtmd.c @@ -197,6 +197,7 @@ struct qtmd_stream *qtmd_init(struct mspack_system *system, /* Quantum supports window sizes of 2^10 (1Kb) through 2^21 (2Mb) */ if (window_bits < 10 || window_bits > 21) return NULL; + /* round up input buffer size to multiple of two */ input_buffer_size = (input_buffer_size + 1) & -2; if (input_buffer_size < 2) return NULL;