Redo the bounds checks in MsiGetFileVersion.

Now any failing bounds check causes the whole function to fail, rather
than returning a made-up value.

3 files changed, 58 insertions, 51 deletions

diff --git a/fake-lib.c b/fake-lib.c
index 9862020..36c6198 100644
--- a/fake-lib.c
+++ b/fake-lib.c
@@ -113,15 +113,3 @@ char *dupcat(const char *str, ...)
 
     return out;
 }
-
-unsigned le(const unsigned char *buf, size_t len, size_t off, size_t nbytes)
-{
-    unsigned toret = 0;
-    off += nbytes;
-    while (nbytes-- > 0) {
-        toret <<= 8;
-        if (--off < len)
-            toret |= buf[off];
-    }
-    return toret;
-}
diff --git a/fake-lib.h b/fake-lib.h
index 8f8774c..de0960f 100644
--- a/fake-lib.h
+++ b/fake-lib.h
@@ -6,4 +6,3 @@ void system_argv(const char *cmd, ...);
 void system_argv_array(char **args);
 void c16cpy(char16_t *out, uint32_t *outsize, char *s);
 char *dupcat(const char *str, ...);
-unsigned le(const unsigned char *buf, size_t len, size_t off, size_t nbytes);
diff --git a/version.c b/version.c
index dd62fc4..c8d4991 100644
--- a/version.c
+++ b/version.c
@@ -38,28 +38,48 @@ uint32_t MsiGetFileVersionW(const char16_t *filename,
         err(1, "%s: mmap", fname);
     unsigned char *map = (unsigned char *)mapv;
 
-    if (le(map, fsize, 0, 2) != (('Z'<<8) | 'M')) {
+#define BYTE(offset) ({                                                 \
+            size_t _offset = (offset);                                  \
+            if (_offset >= fsize) goto cleanup; /* outside file bounds */ \
+            map[_offset];                                               \
+        })
+#define WORD16(offset) ({                               \
+            size_t _offset = (offset);                  \
+            uint16_t toret = BYTE(_offset+1);           \
+            toret = (toret << 8) | BYTE(_offset+0);     \
+            toret;                                      \
+        })
+#define WORD32(offset) ({                               \
+            size_t _offset = (offset);                  \
+            uint16_t toret = BYTE(_offset+3);           \
+            toret = (toret << 8) | BYTE(_offset+2);     \
+            toret = (toret << 8) | BYTE(_offset+1);     \
+            toret = (toret << 8) | BYTE(_offset+0);     \
+            toret;                                      \
+        })
+
+    if (WORD16(0) != (('Z'<<8) | 'M')) {
         warnx("MsiGetFileInfo(%s) -> no MZ", fname);
         goto cleanup;
     }
-    unsigned pe_pos = le(map, fsize, 0x3c, 4);
-    if (le(map, fsize, pe_pos, 4) != (('E'<<8) | 'P')) {
+    unsigned pe_pos = WORD32(0x3c);
+    if (WORD32(pe_pos) != (('E'<<8) | 'P')) {
         warnx("MsiGetFileInfo(%s) -> no PE", fname);
         goto cleanup;
     }
     pe_pos += 4;                       /* skip to the main header */
-    unsigned nsections = le(map, fsize, pe_pos + 2, 2);
-    unsigned opthdr_size = le(map, fsize, pe_pos + 16, 2);
+    unsigned nsections = WORD16(pe_pos + 2);
+    unsigned opthdr_size = WORD16(pe_pos + 16);
     unsigned opthdr_pos = pe_pos + 20;
-    /* bool sixtyfourbit = le(map, fsize, opthdr_pos, 2) == 0x020B; */
+    /* bool sixtyfourbit = WORD16(opthdr_pos) == 0x020B; */
     unsigned secthdr_pos = opthdr_pos + opthdr_size;
     while (nsections > 0) {
-        if (le(map, fsize, secthdr_pos+0, 1) == '.' &&
-            le(map, fsize, secthdr_pos+1, 1) == 'r' &&
-            le(map, fsize, secthdr_pos+2, 1) == 's' &&
-            le(map, fsize, secthdr_pos+3, 1) == 'r' &&
-            le(map, fsize, secthdr_pos+4, 1) == 'c' &&
-            le(map, fsize, secthdr_pos+5, 1) == 0)
+        if (BYTE(secthdr_pos+0) == '.' &&
+            BYTE(secthdr_pos+1) == 'r' &&
+            BYTE(secthdr_pos+2) == 's' &&
+            BYTE(secthdr_pos+3) == 'r' &&
+            BYTE(secthdr_pos+4) == 'c' &&
+            BYTE(secthdr_pos+5) == 0)
             goto found_resource_section;
         secthdr_pos += 0x28;
         nsections--;
@@ -68,17 +88,17 @@ uint32_t MsiGetFileVersionW(const char16_t *filename,
     goto cleanup;
 
   found_resource_section:;
-    unsigned rsrc_size = le(map, fsize, secthdr_pos+8, 4);
-    unsigned rsrc_offset = le(map, fsize, secthdr_pos+20, 4);
-    unsigned rsrc_vaddr = le(map, fsize, secthdr_pos+12, 4);
+    unsigned rsrc_size = WORD32(secthdr_pos+8);
+    unsigned rsrc_offset = WORD32(secthdr_pos+20);
+    unsigned rsrc_vaddr = WORD32(secthdr_pos+12);
 
     unsigned res_dir_offset = rsrc_offset;
     unsigned nnamed, nid;
-    nnamed = le(map, fsize, res_dir_offset+12, 2);
-    nid = le(map, fsize, res_dir_offset+14, 2);
+    nnamed = WORD16(res_dir_offset+12);
+    nid = WORD16(res_dir_offset+14);
     for (unsigned i = nnamed; i < nnamed+nid; i++) {
-        unsigned id = le(map, fsize, res_dir_offset + 16 + 8*i, 4);
-        unsigned entry = le(map, fsize, res_dir_offset + 16 + 8*i + 4, 4);
+        unsigned id = WORD32(res_dir_offset + 16 + 8*i);
+        unsigned entry = WORD32(res_dir_offset + 16 + 8*i + 4);
         if (id == 16 && (entry & 0x80000000)) {
             res_dir_offset = rsrc_offset + (entry & 0x7FFFFFFF);
             goto found_versioninfo_toplevel;
@@ -88,10 +108,10 @@ uint32_t MsiGetFileVersionW(const char16_t *filename,
     goto cleanup;
 
   found_versioninfo_toplevel:
-    nnamed = le(map, fsize, res_dir_offset+12, 2);
-    nid = le(map, fsize, res_dir_offset+14, 2);
+    nnamed = WORD16(res_dir_offset+12);
+    nid = WORD16(res_dir_offset+14);
     for (unsigned i = 0; i < nnamed+nid; i++) {
-        unsigned entry = le(map, fsize, res_dir_offset + 16 + 8*i + 4, 4);
+        unsigned entry = WORD32(res_dir_offset + 16 + 8*i + 4);
         if (entry & 0x80000000) {
             res_dir_offset = rsrc_offset + (entry & 0x7FFFFFFF);
             goto found_versioninfo_2ndlevel;
@@ -101,10 +121,10 @@ uint32_t MsiGetFileVersionW(const char16_t *filename,
     goto cleanup;
 
   found_versioninfo_2ndlevel:
-    nnamed = le(map, fsize, res_dir_offset+12, 2);
-    nid = le(map, fsize, res_dir_offset+14, 2);
+    nnamed = WORD16(res_dir_offset+12);
+    nid = WORD16(res_dir_offset+14);
     for (unsigned i = 0; i < nnamed+nid; i++) {
-        unsigned entry = le(map, fsize, res_dir_offset + 16 + 8*i + 4, 4);
+        unsigned entry = WORD32(res_dir_offset + 16 + 8*i + 4);
         if (!(entry & 0x80000000)) {
             res_dir_offset = rsrc_offset + (entry & 0x7FFFFFFF);
             goto found_versioninfo_3rdlevel;
@@ -114,33 +134,33 @@ uint32_t MsiGetFileVersionW(const char16_t *filename,
     goto cleanup;
 
   found_versioninfo_3rdlevel:;
-    unsigned versioninfo_offset = le(map, fsize, res_dir_offset, 4);
-    unsigned versioninfo_size = le(map, fsize, res_dir_offset+4, 4);
+    unsigned versioninfo_offset = WORD32(res_dir_offset);
+    unsigned versioninfo_size = WORD32(res_dir_offset+4);
     versioninfo_offset = rsrc_offset + versioninfo_offset - rsrc_vaddr;
 
     unsigned name_offset = versioninfo_offset + 6;
     const char *next_name_chr = "VS_VERSION_INFO";
     do {
-        if (le(map, fsize, name_offset, 2) != *next_name_chr)
+        if (WORD16(name_offset) != *next_name_chr)
             goto cleanup;              /* identifying string didn't match */
         name_offset += 2;
     } while (*next_name_chr++);
     unsigned fixed_offset = (name_offset + 3) & ~3;
-    if (le(map, fsize, fixed_offset, 4) != 0xFEEF04BDU) {
+    if (WORD32(fixed_offset) != 0xFEEF04BDU) {
         warnx("MsiGetFileInfo(%s) -> no VS_FIXEDFILEINFO magic number", fname);
         goto cleanup;
     }
     int four_part_version[4];
-    four_part_version[0] = le(map, fsize, fixed_offset + 10, 2);
-    four_part_version[1] = le(map, fsize, fixed_offset +  8, 2);
-    four_part_version[2] = le(map, fsize, fixed_offset + 14, 2);
-    four_part_version[3] = le(map, fsize, fixed_offset + 12, 2);
+    four_part_version[0] = WORD16(fixed_offset + 10);
+    four_part_version[1] = WORD16(fixed_offset +  8);
+    four_part_version[2] = WORD16(fixed_offset + 14);
+    four_part_version[3] = WORD16(fixed_offset + 12);
     unsigned child_offset = fixed_offset +
-        le(map, fsize, versioninfo_offset+2, 2);
+        WORD16(versioninfo_offset+2);
     unsigned lcid;
     while (child_offset < versioninfo_offset + versioninfo_size) {
         unsigned this_child_offset = child_offset;
-        child_offset += le(map, fsize, child_offset, 2);
+        child_offset += WORD16(child_offset);
         child_offset = (child_offset + 3) &~ 3;
         if (child_offset <= this_child_offset) {
             warnx("MsiGetFileInfo(%s) -> bad length field", fname);
@@ -149,7 +169,7 @@ uint32_t MsiGetFileVersionW(const char16_t *filename,
         const char *next_name_chr = "VarFileInfo";
         name_offset = this_child_offset + 6;
         do {
-            if (le(map, fsize, name_offset, 2) != *next_name_chr)
+            if (WORD16(name_offset) != *next_name_chr)
                 goto this_is_not_a_varfileinfo;
             name_offset += 2;
         } while (*next_name_chr++);
@@ -158,14 +178,14 @@ uint32_t MsiGetFileVersionW(const char16_t *filename,
         next_name_chr = "Translation";
         name_offset = subchild_offset + 6;
         do {
-            if (le(map, fsize, name_offset, 2) != *next_name_chr) {
+            if (WORD16(name_offset) != *next_name_chr) {
                 warnx("MsiGetFileInfo(%s) -> child not called 'Translation'", fname);
                 goto cleanup;
             }
             name_offset += 2;
         } while (*next_name_chr++);
         subchild_offset = (name_offset + 3) & ~3;
-        lcid = le(map, fsize, subchild_offset, 2);
+        lcid = WORD16(subchild_offset);
         goto success;
       this_is_not_a_varfileinfo:
         continue;