CVE-2018-14679: Off-by-one Error (CWE-193)
value: An issue was discovered in mspack/chmd.c in libmspack before 0.7alpha. There is an off-by-one error in the CHM PMGI/PMGL chunk number validity checks, which could lead to denial of service (uninitialized data dereference and application crash).
CWE description: A product calculates or uses an incorrect maximum or minimum value that is 1 more, or 1 less, than the correct value.
CVSS v3 base score: 6.5
Published: 2018-07-28T23:29Z
Repository: https://github.com/kyz/libmspack
Language: Other
Files changed: libmspack/ChangeLog, libmspack/mspack/chmd.c
Commit date: 2018-05-12 10:51:34 +0100
Commit message: Fix off-by-one bounds check on CHM PMGI/PMGL chunk numbers and
reject empty filenames. Thanks to Hanno Böck for reporting

Diff: diff --git a/libmspack/ChangeLog b/libmspack/ChangeLog
index 94e4e84..fab3a55 100644
--- a/libmspack/ChangeLog
+++ b/libmspack/ChangeLog
@@ -1,5 +1,15 @@
+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
 	by Dmitry Glavatskikh. Thanks Dmitry!
 
diff --git a/libmspack/mspack/chmd.c b/libmspack/mspack/chmd.c
index c921c8c..9c32658 100644
--- a/libmspack/mspack/chmd.c
+++ b/libmspack/mspack/chmd.c
@@ -1,7 +1,7 @@
 /* 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
  *
  * For further details, see the file COPYING.LIB distributed with libmspack
@@ -395,11 +395,11 @@ static int chmd_read_headers(struct mspack_system *sys, struct mspack_file *fh,
   }
   if (chm->first_pmgl > chm->last_pmgl) {
     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;
   }
 
   /* if we are doing a quick read, stop here! */
@@ -445,11 +445,14 @@ static int chmd_read_headers(struct mspack_system *sys, struct mspack_file *fh,
     num_entries = EndGetI16(end);
 
     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);
 
       /* empty files and directory names are stored as a file entry at
@@ -620,11 +623,11 @@ static unsigned char *read_chunk(struct mschm_decompressor_p *self,
 {
     struct mspack_system *sys = self->system;
     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) {
 	size_t size = sizeof(unsigned char *) * chm->num_chunks;
 	if (!(chm->chunk_cache = (unsigned char **) sys->alloc(sys, size))) {

Vulnerable code:  * (C) 2003-2011 Stuart Caie.
  if (chm->index_root != 0xFFFFFFFF && chm->index_root > chm->num_chunks) {
    if (chunk_num > chm->num_chunks) return NULL;
Fixed code: 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.

 * (C) 2003-2018 Stuart Caie.
  if (chm->index_root != 0xFFFFFFFF && chm->index_root >= chm->num_chunks) {
      /* consider blank filenames to be an error */
      if (name_len == 0) goto chunk_end;

    if (chunk_num >= chm->num_chunks) return NULL;