From 6ba68a64551d40804a426b1b6a2a60313c23c651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3=20Moln=C3=A1r?= Date: Thu, 14 May 2015 21:46:52 +0200 Subject: [PATCH] PE related fixes for CERT-FI 829767 --- src/pefile.cpp | 40 ++++++++++++++++++++++++++++++++++++---- src/pefile.h | 8 +++++++- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/pefile.cpp b/src/pefile.cpp index f607f1a4..1d3755cf 100644 --- a/src/pefile.cpp +++ b/src/pefile.cpp @@ -1493,10 +1493,13 @@ struct PeFile::Resource::upx_rleaf : public PeFile::Resource::upx_rnode PeFile::Resource::Resource() : root(NULL) {} -PeFile::Resource::Resource(const upx_byte *p) +PeFile::Resource::Resource(const upx_byte *p, + const upx_byte *ibufstart_, + const upx_byte *ibufend_) { + ibufstart = ibufstart_; + ibufend = ibufend_; init(p); - } PeFile::Resource::~Resource() @@ -1578,14 +1581,24 @@ void PeFile::Resource::init(const upx_byte *res) void PeFile::Resource::check(const res_dir *node,unsigned level) { + ibufcheck(node, sizeof(*node)); int ic = node->identr + node->namedentr; if (ic == 0) return; for (const res_dir_entry *rde = node->entries; --ic >= 0; rde++) + { + ibufcheck(rde, sizeof(*rde)); if (((rde->child & 0x80000000) == 0) ^ (level == 2)) throwCantPack("unsupported resource structure"); else if (level != 2) check((const res_dir*) (start + (rde->child & 0x7fffffff)),level + 1); + } +} + +void PeFile::Resource::ibufcheck(const void *m, unsigned siz) +{ + if (m < ibufstart || m > ibufend - siz) + throwCantUnpack("corrupted resources"); } PeFile::Resource::upx_rnode *PeFile::Resource::convert(const void *rnode, @@ -1595,6 +1608,7 @@ PeFile::Resource::upx_rnode *PeFile::Resource::convert(const void *rnode, if (level == 3) { const res_data *node = (const res_data *) rnode; + ibufcheck(node, sizeof(*node)); upx_rleaf *leaf = new upx_rleaf; leaf->name = NULL; leaf->parent = parent; @@ -1608,6 +1622,7 @@ PeFile::Resource::upx_rnode *PeFile::Resource::convert(const void *rnode, } const res_dir *node = (const res_dir *) rnode; + ibufcheck(node, sizeof(*node)); int ic = node->identr + node->namedentr; if (ic == 0) return NULL; @@ -1628,7 +1643,9 @@ PeFile::Resource::upx_rnode *PeFile::Resource::convert(const void *rnode, if (child->id & 0x80000000) { const upx_byte *p = start + (child->id & 0x7fffffff); + ibufcheck(p, 2); const unsigned len = 2 + 2 * get_le16(p); + ibufcheck(p, len); child->name = new upx_byte[len]; memcpy(child->name,p,len); // copy unicode string ssize += len; // size of unicode strings @@ -1643,6 +1660,9 @@ void PeFile::Resource::build(const upx_rnode *node, unsigned &bpos, { if (level == 3) { + if (bpos + sizeof(res_data) >= dirsize()) + throwCantUnpack("corrupted resources"); + res_data *l = (res_data*) (newstart + bpos); const upx_rleaf *leaf = (const upx_rleaf*) node; *l = leaf->data; @@ -1651,6 +1671,9 @@ void PeFile::Resource::build(const upx_rnode *node, unsigned &bpos, bpos += sizeof(*l); return; } + if (bpos + sizeof(res_dir) >= dirsize()) + throwCantUnpack("corrupted resources"); + res_dir * const b = (res_dir*) (newstart + bpos); const upx_rbranch *branch = (const upx_rbranch*) node; *b = branch->data; @@ -1666,6 +1689,8 @@ void PeFile::Resource::build(const upx_rnode *node, unsigned &bpos, if ((p = branch->children[ic]->name) != 0) { be->tnl = spos + 0x80000000; + if (spos + get_le16(p) * 2 + 2 >= dirsize()) + throwCantUnpack("corrupted resources"); memcpy(newstart + spos,p,get_le16(p) * 2 + 2); spos += get_le16(p) * 2 + 2; } @@ -2629,11 +2654,16 @@ void PeFile::rebuildResources(upx_byte *& extrainfo, unsigned lastvaddr) extrainfo += 2; const unsigned vaddr = IDADDR(PEDIR_RESOURCE); + + if (lastvaddr > vaddr || (vaddr - lastvaddr) > ibuf.getSize()) + throwCantUnpack("corrupted PE header"); + const upx_byte *r = ibuf - lastvaddr; - Resource res(r + vaddr); + Resource res(r + vaddr, ibuf, ibuf + ibuf.getSize()); while (res.next()) if (res.offs() > vaddr) { + ICHECK(r + res.offs() - 4, 4); unsigned origoffs = get_le32(r + res.offs() - 4); res.newoffs() = origoffs; omemcpy(obuf + origoffs - rvamin,r + res.offs(),res.size()); @@ -2771,6 +2801,8 @@ void PeFile::unpack0(OutputFile *fo, const ht &ih, ht &oh, //infoHeader("[Processing %s, format %s, %d sections]", fn_basename(fi->getName()), getName(), objs); handleStub(fi,fo,pe_offset); + if (ih.filealign == 0) + throwCantUnpack("unexpected value in the PE header"); const unsigned iobjs = ih.objects; const unsigned overlay = file_size - ALIGN_UP(isection[iobjs - 1].rawdataptr @@ -2792,7 +2824,7 @@ void PeFile::unpack0(OutputFile *fo, const ht &ih, ht &oh, extrainfo += sizeof (oh); unsigned objs = oh.objects; - if ((int) objs <= 0) + if ((int) objs <= 0 || isection[2].size == 0) throwCantUnpack("unexpected value in the PE header"); Array(pe_section_t, osection, objs); memcpy(osection,extrainfo,sizeof(pe_section_t) * objs); diff --git a/src/pefile.h b/src/pefile.h index 937cc4dd..25578b90 100644 --- a/src/pefile.h +++ b/src/pefile.h @@ -340,6 +340,9 @@ protected: unsigned dsize; unsigned ssize; + const upx_byte *ibufstart; + const upx_byte *ibufend; + void check(const res_dir*,unsigned); upx_rnode *convert(const void *,upx_rnode *,unsigned); void build(const upx_rnode *,unsigned &,unsigned &,unsigned); @@ -347,9 +350,12 @@ protected: void dump(const upx_rnode *,unsigned) const; void destroy(upx_rnode *urd,unsigned level); + void ibufcheck(const void *m, unsigned size); + public: Resource(); - Resource(const upx_byte *p); + Resource(const upx_byte *p, const upx_byte *ibufstart, + const upx_byte *ibufend); ~Resource(); void init(const upx_byte *);