CVE-2019-1010305: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: libmspack 0.9.1alpha is affected by: Buffer Overflow. The impact is: Information Disclosure. The component is: function chmd_read_headers() in libmspack(file libmspack/mspack/chmd.c). The attack vector is: the victim must open a specially crafted chm file. The fixed version is: after commit 2f084136cfe0d05e5bf5703f3e83c6d955234b4d.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 5.5
Published: 2019-07-15T15:15Z
Repository: https://github.com/kyz/libmspack
Language: Other
Files changed: libmspack/ChangeLog, libmspack/mspack/chmd.c
Commit date: 2019-02-18 13:04:58 +0000
Commit message: length checks when looking for control files

Diff: diff --git a/libmspack/ChangeLog b/libmspack/ChangeLog
index 6e76935..01def2a 100644
--- a/libmspack/ChangeLog
+++ b/libmspack/ChangeLog
@@ -1,5 +1,13 @@
+2019-02-18  Stuart Caie <kyzer@cabextract.org.uk>
+
+	* chmd_read_headers(): a CHM file name beginning "::" but shorter
+	than 33 bytes will lead to reading past the freshly-allocated name
+	buffer - checks for specific control filenames didn't take length
+	into account. Thanks to ADLab of Venustech for the report and
+	proof of concept.
+
 2018-11-03  Stuart Caie <kyzer@cabextract.org.uk>
 
 	* configure.ac, doc/Makefile.in, doc/Doxyfile.in: remove these
 	template files and replace with static files. You can still build
 	the documentation with make -C doc
diff --git a/libmspack/mspack/chmd.c b/libmspack/mspack/chmd.c
index cd947e5..232cf8c 100644
--- a/libmspack/mspack/chmd.c
+++ b/libmspack/mspack/chmd.c
@@ -481,23 +481,21 @@ static int chmd_read_headers(struct mspack_system *sys, struct mspack_file *fh,
       sys->copy(name, fi->filename, (size_t) name_len);
       fi->filename[name_len] = '\0';
 
       if (name[0] == ':' && name[1] == ':') {
         /* system file */
-        if (memcmp(&name[2], &content_name[2], 31L) == 0) {
-          if (memcmp(&name[33], &content_name[33], 8L) == 0) {
-            chm->sec1.content = fi;
-          }
-          else if (memcmp(&name[33], &control_name[33], 11L) == 0) {
-            chm->sec1.control = fi;
-          }
-          else if (memcmp(&name[33], &spaninfo_name[33], 8L) == 0) {
-            chm->sec1.spaninfo = fi;
-          }
-          else if (memcmp(&name[33], &rtable_name[33], 72L) == 0) {
-            chm->sec1.rtable = fi;
-          }
+        if (name_len == 40 && memcmp(name, content_name, 40) == 0) {
+          chm->sec1.content = fi;
+        }
+        else if (name_len == 44 && memcmp(name, control_name, 44) == 0) {
+          chm->sec1.control = fi;
+        }
+        else if (name_len == 41 && memcmp(name, spaninfo_name, 41) == 0) {
+          chm->sec1.spaninfo = fi;
+        }
+        else if (name_len == 105 && memcmp(name, rtable_name, 105) == 0) {
+          chm->sec1.rtable = fi;
         }
         fi->next = chm->sysfiles;
         chm->sysfiles = fi;
       }
       else {

Vulnerable code:         if (memcmp(&name[2], &content_name[2], 31L) == 0) {
          if (memcmp(&name[33], &content_name[33], 8L) == 0) {
            chm->sec1.content = fi;
          }
          else if (memcmp(&name[33], &control_name[33], 11L) == 0) {
            chm->sec1.control = fi;
          }
          else if (memcmp(&name[33], &spaninfo_name[33], 8L) == 0) {
            chm->sec1.spaninfo = fi;
          }
          else if (memcmp(&name[33], &rtable_name[33], 72L) == 0) {
            chm->sec1.rtable = fi;
          }
Fixed code: 2019-02-18  Stuart Caie <kyzer@cabextract.org.uk>

	* chmd_read_headers(): a CHM file name beginning "::" but shorter
	than 33 bytes will lead to reading past the freshly-allocated name
	buffer - checks for specific control filenames didn't take length
	into account. Thanks to ADLab of Venustech for the report and
	proof of concept.

        if (name_len == 40 && memcmp(name, content_name, 40) == 0) {
          chm->sec1.content = fi;
        }
        else if (name_len == 44 && memcmp(name, control_name, 44) == 0) {
          chm->sec1.control = fi;
        }
        else if (name_len == 41 && memcmp(name, spaninfo_name, 41) == 0) {
          chm->sec1.spaninfo = fi;
        }
        else if (name_len == 105 && memcmp(name, rtable_name, 105) == 0) {
          chm->sec1.rtable = fi;