Skip to content

Commit

Permalink
Fix off-by-one bounds check on CHM PMGI/PMGL chunk numbers and
Browse files Browse the repository at this point in the history
reject empty filenames. Thanks to Hanno Böck for reporting
  • Loading branch information
kyz committed May 12, 2018
1 parent 631829c commit 72e70a9
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
10 changes: 10 additions & 0 deletions libmspack/ChangeLog
@@ -1,3 +1,13 @@
2018-04-26 Stuart Caie <kyzer@cabextract.org.uk>

* read_chunk(): the test that chunk numbers are in bounds was off
by one, so read_chunk() returned a pointer taken from outside
allocated memory that usually crashes libmspack when accessed.
Thanks to Hanno Böck for finding the issue and providing a sample.

* chmd_read_headers(): reject files with blank filenames. Thanks
again to Hanno Böck for finding the issue and providing a sample file.

2018-02-06 Stuart Caie <kyzer@cabextract.org.uk>

* chmd.c: fixed an off-by-one error in the TOLOWER() macro, reported
Expand Down
9 changes: 6 additions & 3 deletions libmspack/mspack/chmd.c
@@ -1,5 +1,5 @@
/* This file is part of libmspack.
* (C) 2003-2011 Stuart Caie.
* (C) 2003-2018 Stuart Caie.
*
* libmspack is free software; you can redistribute it and/or modify it under
* the terms of the GNU Lesser General Public License (LGPL) version 2.1
Expand Down Expand Up @@ -397,7 +397,7 @@ static int chmd_read_headers(struct mspack_system *sys, struct mspack_file *fh,
D(("first pmgl chunk is after last pmgl chunk"))
return MSPACK_ERR_DATAFORMAT;
}
if (chm->index_root != 0xFFFFFFFF && chm->index_root > chm->num_chunks) {
if (chm->index_root != 0xFFFFFFFF && chm->index_root >= chm->num_chunks) {
D(("index_root outside valid range"))
return MSPACK_ERR_DATAFORMAT;
}
Expand Down Expand Up @@ -447,7 +447,10 @@ static int chmd_read_headers(struct mspack_system *sys, struct mspack_file *fh,
while (num_entries--) {
READ_ENCINT(name_len);
if (name_len > (unsigned int) (end - p)) goto chunk_end;
/* consider blank filenames to be an error */
if (name_len == 0) goto chunk_end;
name = p; p += name_len;

READ_ENCINT(section);
READ_ENCINT(offset);
READ_ENCINT(length);
Expand Down Expand Up @@ -622,7 +625,7 @@ static unsigned char *read_chunk(struct mschm_decompressor_p *self,
unsigned char *buf;

/* check arguments - most are already checked by chmd_fast_find */
if (chunk_num > chm->num_chunks) return NULL;
if (chunk_num >= chm->num_chunks) return NULL;

/* ensure chunk cache is available */
if (!chm->chunk_cache) {
Expand Down

0 comments on commit 72e70a9

Please sign in to comment.