CVE-2018-14681: Out-of-bounds Write (CWE-787)
value: An issue was discovered in kwajd_read_headers in mspack/kwajd.c in libmspack before 0.7alpha. Bad KWAJ file header extensions could cause a one or two byte overwrite.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 8.8
Published: 2018-07-28T23:29Z
Repository: https://github.com/kyz/libmspack
Language: Other
Files changed: libmspack/ChangeLog, libmspack/Makefile.am, libmspack/mspack/kwajd.c, libmspack/test/.gitignore, libmspack/test/kwajd_test.c, libmspack/test/test_files/kwajd/make.pl
Commit date: 2017-11-26 14:28:54 +0000
Commit message: kwaj_read_headers(): fix handling of non-terminated strings

Diff: diff --git a/libmspack/ChangeLog b/libmspack/ChangeLog
index 4aef85f..44cf81a 100644
--- a/libmspack/ChangeLog
+++ b/libmspack/ChangeLog
@@ -1,5 +1,13 @@
+2017-11-26  Stuart Caie <kyzer@cabextract.org.uk>
+
+	* kwajd_read_headers(): fix up the logic of reading the filename and
+	extension headers to avoid a one or two byte overwrite. Thanks to
+	Jakub Wilk for finding the issue.
+
+	* test/kwajd_test.c: add tests for KWAJ filename.ext handling
+
 2017-10-16  Stuart Caie <kyzer@cabextract.org.uk>
 
 	* test/cabd_test.c: update the short string tests to expect not only
 	MSPACK_ERR_DATAFORMAT but also MSPACK_ERR_READ, because of the recent
 	change to cabd_read_string(). Thanks to maitreyee43 for spotting this.
diff --git a/libmspack/Makefile.am b/libmspack/Makefile.am
index d25dd32..e2d0c96 100644
--- a/libmspack/Makefile.am
+++ b/libmspack/Makefile.am
@@ -22,11 +22,11 @@ include_HEADERS =	mspack/mspack.h
 lib_LTLIBRARIES =	libmspack.la
 
 noinst_LTLIBRARIES =	libmscabd.la libmschmd.la
 noinst_PROGRAMS =	examples/cabd_memory examples/multifh test/cabd_md5 \
 			test/cabd_test test/chmd_find test/chmd_md5 \
-			test/chmd_order test/chminfo
+			test/chmd_order test/chminfo test/kwajd_test
 
 libmspack_la_SOURCES =	mspack/mspack.h \
 			mspack/system.h mspack/system.c \
 			mspack/cab.h mspack/cabc.c mspack/cabd.c \
 			mspack/chm.h mspack/chmc.c mspack/chmd.c \
@@ -87,5 +87,7 @@ test_chmd_md5_LDADD =		libmschmd.la
 test_chmd_order_SOURCES =	test/chmd_order.c test/md5.c test/md5.h \
 				test/md5_fh.h test/error.h libmschmd.la
 test_chmd_order_LDADD =		libmschmd.la
 test_chminfo_SOURCES =		test/chminfo.c libmschmd.la
 test_chminfo_LDADD =		libmschmd.la
+test_kwajd_test_SOURCES =	test/kwajd_test.c libmspack.la
+test_kwajd_test_LDADD = 	libmspack.la
diff --git a/libmspack/mspack/kwajd.c b/libmspack/mspack/kwajd.c
index c9e006c..50be257 100644
--- a/libmspack/mspack/kwajd.c
+++ b/libmspack/mspack/kwajd.c
@@ -196,34 +196,40 @@ static int kwajd_read_headers(struct mspack_system *sys,
 	if (sys->seek(fh, (off_t)i, MSPACK_SYS_SEEK_CUR)) return MSPACK_ERR_SEEK;
     }
 
     /* filename and extension */
     if (hdr->headers & (MSKWAJ_HDR_HASFILENAME | MSKWAJ_HDR_HASFILEEXT)) {
-	off_t pos = sys->tell(fh);
-	char *fn = (char *) sys->alloc(sys, (size_t) 13);
-
+	int len;
 	/* allocate memory for maximum length filename */
-	if (! fn) return MSPACK_ERR_NOMEMORY;
-	hdr->filename = fn;
+	char *fn = (char *) sys->alloc(sys, (size_t) 13);
+	if (!(hdr->filename = fn)) return MSPACK_ERR_NOMEMORY;
 
 	/* copy filename if present */
 	if (hdr->headers & MSKWAJ_HDR_HASFILENAME) {
-	    if (sys->read(fh, &buf[0], 9) != 9) return MSPACK_ERR_READ;
-	    for (i = 0; i < 9; i++, fn++) if (!(*fn = buf[i])) break;
-	    pos += (i < 9) ? i+1 : 9;
-	    if (sys->seek(fh, pos, MSPACK_SYS_SEEK_START))
+	    /* read and copy up to 9 bytes of a null terminated string */
+	    if ((len = sys->read(fh, &buf[0], 9)) < 2) return MSPACK_ERR_READ;
+	    for (i = 0; i < len; i++) if (!(*fn++ = buf[i])) break;
+	    /* if string was 9 bytes with no null terminator, reject it */
+	    if (i == 9 && buf[8] != '\0') return MSPACK_ERR_DATAFORMAT;
+	    /* seek to byte after string ended in file */
+	    if (sys->seek(fh, (off_t)(i + 1 - len), MSPACK_SYS_SEEK_CUR))
 		return MSPACK_ERR_SEEK;
+	    fn--; /* remove the null terminator */
 	}
 
 	/* copy extension if present */
 	if (hdr->headers & MSKWAJ_HDR_HASFILEEXT) {
 	    *fn++ = '.';
-	    if (sys->read(fh, &buf[0], 4) != 4) return MSPACK_ERR_READ;
-	    for (i = 0; i < 4; i++, fn++) if (!(*fn = buf[i])) break;
-	    pos += (i < 4) ? i+1 : 4;
-	    if (sys->seek(fh, pos, MSPACK_SYS_SEEK_START))
+	    /* read and copy up to 4 bytes of a null terminated string */
+	    if ((len = sys->read(fh, &buf[0], 4)) < 2) return MSPACK_ERR_READ;
+	    for (i = 0; i < len; i++) if (!(*fn++ = buf[i])) break;
+	    /* if string was 4 bytes with no null terminator, reject it */
+	    if (i == 4 && buf[3] != '\0') return MSPACK_ERR_DATAFORMAT;
+	    /* seek to byte after string ended in file */
+	    if (sys->seek(fh, (off_t)(i + 1 - len), MSPACK_SYS_SEEK_CUR))
 		return MSPACK_ERR_SEEK;
+	    fn--; /* remove the null terminator */
 	}
 	*fn = '\0';
     }
 
     /* 2 bytes: extra text length then [length] bytes of extra text data */
diff --git a/libmspack/test/.gitignore b/libmspack/test/.gitignore
index 47414e5..ee9c6d9 100644
--- a/libmspack/test/.gitignore
+++ b/libmspack/test/.gitignore
@@ -6,5 +6,6 @@ cabd_md5
 cabd_test
 chmd_find
 chmd_md5
 chmd_order
 chminfo
+kwajd_test
diff --git a/libmspack/test/kwajd_test.c b/libmspack/test/kwajd_test.c
new file mode 100644
index 0000000..4ee10f9
--- /dev/null
+++ b/libmspack/test/kwajd_test.c
@@ -0,0 +1,116 @@
+/* KWAJ regression test suite */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <mspack.h>
+
+unsigned int test_count = 0;
+#define TEST(x) do {\
+    test_count++; \
+    if (!(x)) {printf("%s:%d FAILED %s\n",__FUNCTION__,__LINE__,#x);exit(1);} \
+} while (0)
+
+/* test parsing of KWAJ filename/extension headers */
+void kwajd_open_test_01() {
+    struct mskwaj_decompressor *kwajd;
+    struct mskwajd_header *hdr;
+
+    kwajd = mspack_create_kwaj_decompressor(NULL);
+    TEST(kwajd != NULL);
+
+    hdr = kwajd->open(kwajd, "test_files/kwajd/f00.kwj");
+    TEST(hdr != NULL);
+    TEST(hdr->filename == NULL);
+    kwajd->close(kwajd, hdr);
+
+#define TEST_FNAME(testfile, fname)      \
+    hdr = kwajd->open(kwajd, testfile);  \
+    TEST(hdr != NULL);                   \
+    TEST(hdr->filename != NULL);         \
+    TEST(!strcmp(fname, hdr->filename)); \
+    kwajd->close(kwajd, hdr)
+#define TEST_FNAME_BAD(testfile)         \
+    hdr = kwajd->open(kwajd, testfile);  \
+    TEST(hdr == NULL);                   \
+    TEST(kwajd->last_error(kwajd) == MSPACK_ERR_DATAFORMAT)
+
+    TEST_FNAME("test_files/kwajd/f01.kwj", ".1");
+    TEST_FNAME("test_files/kwajd/f02.kwj", ".12");
+    TEST_FNAME("test_files/kwajd/f03.kwj", ".123");
+
+    TEST_FNAME("test_files/kwajd/f10.kwj", "1");
+    TEST_FNAME("test_files/kwajd/f11.kwj", "1.1");
+    TEST_FNAME("test_files/kwajd/f12.kwj", "1.12");
+    TEST_FNAME("test_files/kwajd/f13.kwj", "1.123");
+
+    TEST_FNAME("test_files/kwajd/f20.kwj", "12");
+    TEST_FNAME("test_files/kwajd/f21.kwj", "12.1");
+    TEST_FNAME("test_files/kwajd/f22.kwj", "12.12");
+    TEST_FNAME("test_files/kwajd/f23.kwj", "12.123");
+
+    TEST_FNAME("test_files/kwajd/f30.kwj", "123");
+    TEST_FNAME("test_files/kwajd/f31.kwj", "123.1");
+    TEST_FNAME("test_files/kwajd/f32.kwj", "123.12");
+    TEST_FNAME("test_files/kwajd/f33.kwj", "123.123");
+
+    TEST_FNAME("test_files/kwajd/f40.kwj", "1234");
+    TEST_FNAME("test_files/kwajd/f41.kwj", "1234.1");
+    TEST_FNAME("test_files/kwajd/f42.kwj", "1234.12");
+    TEST_FNAME("test_files/kwajd/f43.kwj", "1234.123");
+
+    TEST_FNAME("test_files/kwajd/f50.kwj", "12345");
+    TEST_FNAME("test_files/kwajd/f51.kwj", "12345.1");
+    TEST_FNAME("test_files/kwajd/f52.kwj", "12345.12");
+    TEST_FNAME("test_files/kwajd/f53.kwj", "12345.123");
+
+    TEST_FNAME("test_files/kwajd/f60.kwj", "123456");
+    TEST_FNAME("test_files/kwajd/f61.kwj", "123456.1");
+    TEST_FNAME("test_files/kwajd/f62.kwj", "123456.12");
+    TEST_FNAME("test_files/kwajd/f63.kwj", "123456.123");
+
+    TEST_FNAME("test_files/kwajd/f70.kwj", "1234567");
+    TEST_FNAME("test_files/kwajd/f71.kwj", "1234567.1");
+    TEST_FNAME("test_files/kwajd/f72.kwj", "1234567.12");
+    TEST_FNAME("test_files/kwajd/f73.kwj", "1234567.123");
+
+    TEST_FNAME("test_files/kwajd/f80.kwj", "12345678");
+    TEST_FNAME("test_files/kwajd/f81.kwj", "12345678.1");
+    TEST_FNAME("test_files/kwajd/f82.kwj", "12345678.12");
+    TEST_FNAME("test_files/kwajd/f83.kwj", "12345678.123");
+
+    TEST_FNAME_BAD("test_files/kwajd/f04.kwj");
+    TEST_FNAME_BAD("test_files/kwajd/f14.kwj");
+    TEST_FNAME_BAD("test_files/kwajd/f24.kwj");
+    TEST_FNAME_BAD("test_files/kwajd/f34.kwj");
+    TEST_FNAME_BAD("test_files/kwajd/f44.kwj");
+    TEST_FNAME_BAD("test_files/kwajd/f54.kwj");
+    TEST_FNAME_BAD("test_files/kwajd/f64.kwj");
+    TEST_FNAME_BAD("test_files/kwajd/f74.kwj");
+    TEST_FNAME_BAD("test_files/kwajd/f84.kwj");
+
+    TEST_FNAME_BAD("test_files/kwajd/f90.kwj");
+    TEST_FNAME_BAD("test_files/kwajd/f91.kwj");
+    TEST_FNAME_BAD("test_files/kwajd/f92.kwj");
+    TEST_FNAME_BAD("test_files/kwajd/f93.kwj");
+    TEST_FNAME_BAD("test_files/kwajd/f94.kwj");
+
+
+    mspack_destroy_kwaj_decompressor(kwajd);
+}
+
+int main() {
+  int selftest;
+
+  MSPACK_SYS_SELFTEST(selftest);
+  TEST(selftest == MSPACK_ERR_OK);
+
+  kwajd_open_test_01();
+
+  printf("ALL %d TESTS PASSED.\n", test_count);
+  return 0;
+}
diff --git a/libmspack/test/test_files/kwajd/f00.kwj b/libmspack/test/test_files/kwajd/f00.kwj
new file mode 100644
index 0000000..1aa817e
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f00.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f01.kwj b/libmspack/test/test_files/kwajd/f01.kwj
new file mode 100644
index 0000000..9fbedda
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f01.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f02.kwj b/libmspack/test/test_files/kwajd/f02.kwj
new file mode 100644
index 0000000..a81a863
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f02.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f03.kwj b/libmspack/test/test_files/kwajd/f03.kwj
new file mode 100644
index 0000000..52bfb1e
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f03.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f04.kwj b/libmspack/test/test_files/kwajd/f04.kwj
new file mode 100644
index 0000000..1775433
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f04.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f10.kwj b/libmspack/test/test_files/kwajd/f10.kwj
new file mode 100644
index 0000000..40f944b
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f10.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f11.kwj b/libmspack/test/test_files/kwajd/f11.kwj
new file mode 100644
index 0000000..eca53c9
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f11.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f12.kwj b/libmspack/test/test_files/kwajd/f12.kwj
new file mode 100644
index 0000000..767c355
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f12.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f13.kwj b/libmspack/test/test_files/kwajd/f13.kwj
new file mode 100644
index 0000000..f8d7fb6
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f13.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f14.kwj b/libmspack/test/test_files/kwajd/f14.kwj
new file mode 100644
index 0000000..486e580
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f14.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f20.kwj b/libmspack/test/test_files/kwajd/f20.kwj
new file mode 100644
index 0000000..89de0ef
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f20.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f21.kwj b/libmspack/test/test_files/kwajd/f21.kwj
new file mode 100644
index 0000000..a4c3a8c
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f21.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f22.kwj b/libmspack/test/test_files/kwajd/f22.kwj
new file mode 100644
index 0000000..fa9c35d
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f22.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f23.kwj b/libmspack/test/test_files/kwajd/f23.kwj
new file mode 100644
index 0000000..c10cc71
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f23.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f24.kwj b/libmspack/test/test_files/kwajd/f24.kwj
new file mode 100644
index 0000000..75858e7
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f24.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f30.kwj b/libmspack/test/test_files/kwajd/f30.kwj
new file mode 100644
index 0000000..e2b452e
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f30.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f31.kwj b/libmspack/test/test_files/kwajd/f31.kwj
new file mode 100644
index 0000000..05e0ce1
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f31.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f32.kwj b/libmspack/test/test_files/kwajd/f32.kwj
new file mode 100644
index 0000000..96a5739
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f32.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f33.kwj b/libmspack/test/test_files/kwajd/f33.kwj
new file mode 100644
index 0000000..935d6df
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f33.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f34.kwj b/libmspack/test/test_files/kwajd/f34.kwj
new file mode 100644
index 0000000..d7084c5
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f34.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f40.kwj b/libmspack/test/test_files/kwajd/f40.kwj
new file mode 100644
index 0000000..64cfc84
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f40.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f41.kwj b/libmspack/test/test_files/kwajd/f41.kwj
new file mode 100644
index 0000000..657177a
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f41.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f42.kwj b/libmspack/test/test_files/kwajd/f42.kwj
new file mode 100644
index 0000000..6ff2b22
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f42.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f43.kwj b/libmspack/test/test_files/kwajd/f43.kwj
new file mode 100644
index 0000000..52c6c10
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f43.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f44.kwj b/libmspack/test/test_files/kwajd/f44.kwj
new file mode 100644
index 0000000..bc4d510
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f44.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f50.kwj b/libmspack/test/test_files/kwajd/f50.kwj
new file mode 100644
index 0000000..31bf367
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f50.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f51.kwj b/libmspack/test/test_files/kwajd/f51.kwj
new file mode 100644
index 0000000..c63c061
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f51.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f52.kwj b/libmspack/test/test_files/kwajd/f52.kwj
new file mode 100644
index 0000000..9ab9716
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f52.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f53.kwj b/libmspack/test/test_files/kwajd/f53.kwj
new file mode 100644
index 0000000..e973626
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f53.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f54.kwj b/libmspack/test/test_files/kwajd/f54.kwj
new file mode 100644
index 0000000..f3b2887
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f54.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f60.kwj b/libmspack/test/test_files/kwajd/f60.kwj
new file mode 100644
index 0000000..af39e58
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f60.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f61.kwj b/libmspack/test/test_files/kwajd/f61.kwj
new file mode 100644
index 0000000..a8d6278
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f61.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f62.kwj b/libmspack/test/test_files/kwajd/f62.kwj
new file mode 100644
index 0000000..225f8df
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f62.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f63.kwj b/libmspack/test/test_files/kwajd/f63.kwj
new file mode 100644
index 0000000..50f7980
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f63.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f64.kwj b/libmspack/test/test_files/kwajd/f64.kwj
new file mode 100644
index 0000000..84c2679
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f64.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f70.kwj b/libmspack/test/test_files/kwajd/f70.kwj
new file mode 100644
index 0000000..3507550
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f70.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f71.kwj b/libmspack/test/test_files/kwajd/f71.kwj
new file mode 100644
index 0000000..92c2141
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f71.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f72.kwj b/libmspack/test/test_files/kwajd/f72.kwj
new file mode 100644
index 0000000..a2e57ab
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f72.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f73.kwj b/libmspack/test/test_files/kwajd/f73.kwj
new file mode 100644
index 0000000..77b1cad
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f73.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f74.kwj b/libmspack/test/test_files/kwajd/f74.kwj
new file mode 100644
index 0000000..a760b8e
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f74.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f80.kwj b/libmspack/test/test_files/kwajd/f80.kwj
new file mode 100644
index 0000000..b2e106f
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f80.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f81.kwj b/libmspack/test/test_files/kwajd/f81.kwj
new file mode 100644
index 0000000..6f5bddd
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f81.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f82.kwj b/libmspack/test/test_files/kwajd/f82.kwj
new file mode 100644
index 0000000..eea7493
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f82.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f83.kwj b/libmspack/test/test_files/kwajd/f83.kwj
new file mode 100644
index 0000000..161a940
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f83.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f84.kwj b/libmspack/test/test_files/kwajd/f84.kwj
new file mode 100644
index 0000000..d0e0233
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f84.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f90.kwj b/libmspack/test/test_files/kwajd/f90.kwj
new file mode 100644
index 0000000..8605aca
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f90.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f91.kwj b/libmspack/test/test_files/kwajd/f91.kwj
new file mode 100644
index 0000000..2fadfc7
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f91.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f92.kwj b/libmspack/test/test_files/kwajd/f92.kwj
new file mode 100644
index 0000000..c8a99ae
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f92.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f93.kwj b/libmspack/test/test_files/kwajd/f93.kwj
new file mode 100644
index 0000000..559faa9
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f93.kwj differ
diff --git a/libmspack/test/test_files/kwajd/f94.kwj b/libmspack/test/test_files/kwajd/f94.kwj
new file mode 100644
index 0000000..c52dfeb
Binary files /dev/null and b/libmspack/test/test_files/kwajd/f94.kwj differ
diff --git a/libmspack/test/test_files/kwajd/make.pl b/libmspack/test/test_files/kwajd/make.pl
new file mode 100755
index 0000000..ae73038
--- /dev/null
+++ b/libmspack/test/test_files/kwajd/make.pl
@@ -0,0 +1,17 @@
+#!/usr/bin/perl -w
+use strict;
+my $name = '123456789';
+for my $file (0 .. 9) {
+    for my $ext (0 .. 4) {
+	open my $fh, '>', "f$file$ext.kwj";
+        my $offset = 14  + $file + $ext;
+	my $flags  = ($file > 0 ? 8 : 0) | ($ext > 0 ? 16 : 0);
+	print $fh pack 'A4Vvvv', 'KWAJ', 0xD127F088, 0, $offset, $flags;
+	print $fh substr $name, 0, $file if $file > 0;
+	print $fh "\0" if $file > 0 && $file < 9;
+	print $fh substr $name, 0, $ext if $ext > 0;
+	print $fh "\0" if $ext > 0 && $ext < 4;
+        print $fh "\xFF";
+	close $fh;
+    }
+}

Vulnerable code: 			test/chmd_order test/chminfo
	off_t pos = sys->tell(fh);
	char *fn = (char *) sys->alloc(sys, (size_t) 13);

	if (! fn) return MSPACK_ERR_NOMEMORY;
	hdr->filename = fn;
	    if (sys->read(fh, &buf[0], 9) != 9) return MSPACK_ERR_READ;
	    for (i = 0; i < 9; i++, fn++) if (!(*fn = buf[i])) break;
	    pos += (i < 9) ? i+1 : 9;
	    if (sys->seek(fh, pos, MSPACK_SYS_SEEK_START))
	    if (sys->read(fh, &buf[0], 4) != 4) return MSPACK_ERR_READ;
	    for (i = 0; i < 4; i++, fn++) if (!(*fn = buf[i])) break;
	    pos += (i < 4) ? i+1 : 4;
	    if (sys->seek(fh, pos, MSPACK_SYS_SEEK_START))
Fixed code: 2017-11-26  Stuart Caie <kyzer@cabextract.org.uk>

	* kwajd_read_headers(): fix up the logic of reading the filename and
	extension headers to avoid a one or two byte overwrite. Thanks to
	Jakub Wilk for finding the issue.

	* test/kwajd_test.c: add tests for KWAJ filename.ext handling

			test/chmd_order test/chminfo test/kwajd_test
test_kwajd_test_SOURCES =	test/kwajd_test.c libmspack.la
test_kwajd_test_LDADD = 	libmspack.la
	int len;
	char *fn = (char *) sys->alloc(sys, (size_t) 13);
	if (!(hdr->filename = fn)) return MSPACK_ERR_NOMEMORY;
	    /* read and copy up to 9 bytes of a null terminated string */
	    if ((len = sys->read(fh, &buf[0], 9)) < 2) return MSPACK_ERR_READ;
	    for (i = 0; i < len; i++) if (!(*fn++ = buf[i])) break;
	    /* if string was 9 bytes with no null terminator, reject it */
	    if (i == 9 && buf[8] != '\0') return MSPACK_ERR_DATAFORMAT;
	    /* seek to byte after string ended in file */
	    if (sys->seek(fh, (off_t)(i + 1 - len), MSPACK_SYS_SEEK_CUR))
	    fn--; /* remove the null terminator */
	    /* read and copy up to 4 bytes of a null terminated string */
	    if ((len = sys->read(fh, &buf[0], 4)) < 2) return MSPACK_ERR_READ;
	    for (i = 0; i < len; i++) if (!(*fn++ = buf[i])) break;
	    /* if string was 4 bytes with no null terminator, reject it */
	    if (i == 4 && buf[3] != '\0') return MSPACK_ERR_DATAFORMAT;
	    /* seek to byte after string ended in file */
	    if (sys->seek(fh, (off_t)(i + 1 - len), MSPACK_SYS_SEEK_CUR))
	    fn--; /* remove the null terminator */
kwajd_test
/* KWAJ regression test suite */

#ifdef HAVE_CONFIG_H
#include <config.h>
#endif

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <mspack.h>

unsigned int test_count = 0;
#define TEST(x) do {\
    test_count++; \
    if (!(x)) {printf("%s:%d FAILED %s\n",__FUNCTION__,__LINE__,#x);exit(1);} \
} while (0)

/* test parsing of KWAJ filename/extension headers */
void kwajd_open_test_01() {
    struct mskwaj_decompressor *kwajd;
    struct mskwajd_header *hdr;

    kwajd = mspack_create_kwaj_decompressor(NULL);
    TEST(kwajd != NULL);

    hdr = kwajd->open(kwajd, "test_files/kwajd/f00.kwj");
    TEST(hdr != NULL);
    TEST(hdr->filename == NULL);
    kwajd->close(kwajd, hdr);

#define TEST_FNAME(testfile, fname)      \
    hdr = kwajd->open(kwajd, testfile);  \
    TEST(hdr != NULL);                   \
    TEST(hdr->filename != NULL);         \
    TEST(!strcmp(fname, hdr->filename)); \
    kwajd->close(kwajd, hdr)
#define TEST_FNAME_BAD(testfile)         \
    hdr = kwajd->open(kwajd, testfile);  \
    TEST(hdr == NULL);                   \
    TEST(kwajd->last_error(kwajd) == MSPACK_ERR_DATAFORMAT)

    TEST_FNAME("test_files/kwajd/f01.kwj", ".1");
    TEST_FNAME("test_files/kwajd/f02.kwj", ".12");
    TEST_FNAME("test_files/kwajd/f03.kwj", ".123");

    TEST_FNAME("test_files/kwajd/f10.kwj", "1");
    TEST_FNAME("test_files/kwajd/f11.kwj", "1.1");
    TEST_FNAME("test_files/kwajd/f12.kwj", "1.12");
    TEST_FNAME("test_files/kwajd/f13.kwj", "1.123");

    TEST_FNAME("test_files/kwajd/f20.kwj", "12");
    TEST_FNAME("test_files/kwajd/f21.kwj", "12.1");
    TEST_FNAME("test_files/kwajd/f22.kwj", "12.12");
    TEST_FNAME("test_files/kwajd/f23.kwj", "12.123");

    TEST_FNAME("test_files/kwajd/f30.kwj", "123");
    TEST_FNAME("test_files/kwajd/f31.kwj", "123.1");
    TEST_FNAME("test_files/kwajd/f32.kwj", "123.12");
    TEST_FNAME("test_files/kwajd/f33.kwj", "123.123");

    TEST_FNAME("test_files/kwajd/f40.kwj", "1234");
    TEST_FNAME("test_files/kwajd/f41.kwj", "1234.1");
    TEST_FNAME("test_files/kwajd/f42.kwj", "1234.12");
    TEST_FNAME("test_files/kwajd/f43.kwj", "1234.123");

    TEST_FNAME("test_files/kwajd/f50.kwj", "12345");
    TEST_FNAME("test_files/kwajd/f51.kwj", "12345.1");
    TEST_FNAME("test_files/kwajd/f52.kwj", "12345.12");
    TEST_FNAME("test_files/kwajd/f53.kwj", "12345.123");

    TEST_FNAME("test_files/kwajd/f60.kwj", "123456");
    TEST_FNAME("test_files/kwajd/f61.kwj", "123456.1");
    TEST_FNAME("test_files/kwajd/f62.kwj", "123456.12");
    TEST_FNAME("test_files/kwajd/f63.kwj", "123456.123");

    TEST_FNAME("test_files/kwajd/f70.kwj", "1234567");
    TEST_FNAME("test_files/kwajd/f71.kwj", "1234567.1");
    TEST_FNAME("test_files/kwajd/f72.kwj", "1234567.12");
    TEST_FNAME("test_files/kwajd/f73.kwj", "1234567.123");

    TEST_FNAME("test_files/kwajd/f80.kwj", "12345678");
    TEST_FNAME("test_files/kwajd/f81.kwj", "12345678.1");
    TEST_FNAME("test_files/kwajd/f82.kwj", "12345678.12");
    TEST_FNAME("test_files/kwajd/f83.kwj", "12345678.123");

    TEST_FNAME_BAD("test_files/kwajd/f04.kwj");
    TEST_FNAME_BAD("test_files/kwajd/f14.kwj");
    TEST_FNAME_BAD("test_files/kwajd/f24.kwj");
    TEST_FNAME_BAD("test_files/kwajd/f34.kwj");
    TEST_FNAME_BAD("test_files/kwajd/f44.kwj");
    TEST_FNAME_BAD("test_files/kwajd/f54.kwj");
    TEST_FNAME_BAD("test_files/kwajd/f64.kwj");
    TEST_FNAME_BAD("test_files/kwajd/f74.kwj");
    TEST_FNAME_BAD("test_files/kwajd/f84.kwj");

    TEST_FNAME_BAD("test_files/kwajd/f90.kwj");
    TEST_FNAME_BAD("test_files/kwajd/f91.kwj");
    TEST_FNAME_BAD("test_files/kwajd/f92.kwj");
    TEST_FNAME_BAD("test_files/kwajd/f93.kwj");
    TEST_FNAME_BAD("test_files/kwajd/f94.kwj");


    mspack_destroy_kwaj_decompressor(kwajd);
}

int main() {
  int selftest;

  MSPACK_SYS_SELFTEST(selftest);
  TEST(selftest == MSPACK_ERR_OK);

  kwajd_open_test_01();

  printf("ALL %d TESTS PASSED.\n", test_count);
  return 0;
}
#!/usr/bin/perl -w
use strict;
my $name = '123456789';
for my $file (0 .. 9) {
    for my $ext (0 .. 4) {
	open my $fh, '>', "f$file$ext.kwj";
        my $offset = 14  + $file + $ext;
	my $flags  = ($file > 0 ? 8 : 0) | ($ext > 0 ? 16 : 0);
	print $fh pack 'A4Vvvv', 'KWAJ', 0xD127F088, 0, $offset, $flags;
	print $fh substr $name, 0, $file if $file > 0;
	print $fh "\0" if $file > 0 && $file < 9;
	print $fh substr $name, 0, $ext if $ext > 0;
	print $fh "\0" if $ext > 0 && $ext < 4;
        print $fh "\xFF";
	close $fh;
    }
}