From 69d886c5757e405785ce811d8622f4ff189a9514 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Thu, 18 May 2017 06:57:29 +0100 Subject: Redo the bounds checks in MsiGetFileVersion. Now any failing bounds check causes the whole function to fail, rather than returning a made-up value. --- fake-lib.c | 12 -------- fake-lib.h | 1 - version.c | 96 +++++++++++++++++++++++++++++++++++++------------------------- 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; -- cgit v1.2.3-55-g6feb