From 73307c399643c281d1a1001155d86939faf4e799 Mon Sep 17 00:00:00 2001 From: Moinak Ghosh Date: Sun, 21 Dec 2014 14:13:58 +0530 Subject: [PATCH] Multiple checks and balances in Dispack to avoid buffer overlfow. Allow filter variants to omit the standard header. Use E8E9 in Dispack filter as a fallback. Fix integer overflow for type value in thread data struct. Do not inline functions in DEBUG build. --- archive/dispack_helper.cpp | 50 ++++++++++++++++++++++++++++++++------ archive/pc_arc_filter.c | 14 +++-------- archive/pc_arc_filter.h | 1 + archive/pc_archive.c | 23 ++++++++++-------- config | 1 + filters/dispack/dis.cpp | 19 +++++++++++---- pcompress.c | 2 +- pcompress.h | 2 +- 8 files changed, 77 insertions(+), 35 deletions(-) diff --git a/archive/dispack_helper.cpp b/archive/dispack_helper.cpp index ca2e55b..e975bd5 100644 --- a/archive/dispack_helper.cpp +++ b/archive/dispack_helper.cpp @@ -40,10 +40,12 @@ extern "C" { #endif typedef unsigned char uchar_t; +#define DISPACK_MAGIC "DisPack " #pragma pack(1) struct FileHeader { + char magic[8]; sU32 SizeBefore; // number of bytes before start of code section sU32 SizeAfter; // number of bytes after code section sU32 SizeTransformed; // size of transformed code section @@ -57,6 +59,7 @@ dispack_filter_encode(uchar_t *inData, size_t len, uchar_t **out_buf) { uchar_t *pos; FileHeader hdr; + sU32 sizeNow; *out_buf = (uchar_t *)malloc(len); if (*out_buf == NULL) @@ -95,25 +98,48 @@ dispack_filter_encode(uchar_t *inData, size_t len, uchar_t **out_buf) // transform code sU32 transSize = len - sizeof (hdr); - if (DisFilter(inData + fileOffs, codeSize, imageBase + codeStart, pos, transSize) == NULL) - return (0); + if (fileOffs + codeSize > len) { + codeSize = len - fileOffs; + } + if (DisFilter(inData + fileOffs, codeSize, imageBase + codeStart, pos, transSize) == NULL) { + goto cmp_E89; + } pos += transSize; - // Now plonk the header + // Give up if dispack savings is not enough for header space and we can overflow the buffer. + memcpy(hdr.magic, DISPACK_MAGIC, strlen(DISPACK_MAGIC)); hdr.SizeBefore = fileOffs; hdr.SizeAfter = len - (fileOffs + codeSize); hdr.SizeTransformed = transSize; hdr.SizeOriginal = codeSize; hdr.Origin = imageBase + codeStart; + sizeNow = pos - *out_buf; + if (sizeNow + hdr.SizeBefore + hdr.SizeAfter >= len) { + goto cmp_E89; + } + + // Now plonk the header memcpy(*out_buf, &hdr, sizeof (hdr)); - // Copy rest of the data - memcpy(pos, inData, hdr.SizeBefore); - pos += hdr.SizeBefore; - memcpy(pos, inData + (fileOffs + codeSize), hdr.SizeAfter); - pos += hdr.SizeAfter; + if (hdr.SizeBefore > 0) { + // Copy rest of the data + memcpy(pos, inData, hdr.SizeBefore); + pos += hdr.SizeBefore; + } + if (hdr.SizeAfter > 0) { + memcpy(pos, inData + (fileOffs + codeSize), hdr.SizeAfter); + pos += hdr.SizeAfter; + } return (pos - *out_buf); +cmp_E89: + // Apply an E8E9 filter this does not put the DISPACK_MAGIC into the + // file header. So, when decoding, that is detected and E8E9 decode + // is applied. + memcpy(*out_buf, inData, len); + if (Forward_E89(*out_buf, len) != 0) + return (0); + return (len); } size_t @@ -131,6 +157,9 @@ dispack_filter_decode(uchar_t *inData, size_t len, uchar_t **out_buf) if (*out_buf == NULL) return (0); + if (memcmp(hdr->magic, DISPACK_MAGIC, strlen(DISPACK_MAGIC)) != 0) + goto dec_E89; + decoded = *out_buf; memcpy(decoded, before, hdr->SizeBefore); decoded += hdr->SizeBefore; @@ -144,6 +173,11 @@ dispack_filter_decode(uchar_t *inData, size_t len, uchar_t **out_buf) decoded += hdr->SizeAfter; return (decoded - *out_buf); +dec_E89: + memcpy(*out_buf, inData, len); + if (Inverse_E89(*out_buf, len) == -1) + return (0); + return (len); } #ifdef __cplusplus diff --git a/archive/pc_arc_filter.c b/archive/pc_arc_filter.c index 55b9949..0b3982e 100644 --- a/archive/pc_arc_filter.c +++ b/archive/pc_arc_filter.c @@ -562,14 +562,7 @@ dispack_filter(struct filter_info *fi, void *filter_private) log_msg(LOG_ERR, 0, "Failed to read archive data."); return (FILTER_RETURN_ERROR); } - - /* - * First 8 bytes in the data is the compressed size of the entry. - * LibArchive always zero-pads entries to their original size so - * we need to separately store the compressed size. - */ - in_size = LE64(U64_P(sdat->in_buff)); - mapbuf = sdat->in_buff + 8; + mapbuf = sdat->in_buff; /* * No check for supported EXE types needed here since supported @@ -584,7 +577,7 @@ dispack_filter(struct filter_info *fi, void *filter_private) if (fi->compressing) { out = NULL; len = dispack_filter_encode(mapbuf, len, &out); - if (len == 0 || len >= (len1 - 8)) { + if (len == 0) { munmap(mapbuf, len1); free(out); return (FILTER_RETURN_SKIP); @@ -594,7 +587,8 @@ dispack_filter(struct filter_info *fi, void *filter_private) fi->fout->output_type = FILTER_OUTPUT_MEM; fi->fout->out = out; fi->fout->out_size = len; - fi->fout->hdr.in_size = LE64(len1); + fi->fout->hdr_valid = 0; + *(fi->type_ptr) = TYPE_UNKNOWN; return (ARCHIVE_OK); } diff --git a/archive/pc_arc_filter.h b/archive/pc_arc_filter.h index 8978a64..db4bea7 100644 --- a/archive/pc_arc_filter.h +++ b/archive/pc_arc_filter.h @@ -71,6 +71,7 @@ typedef struct _filter_output { uint8_t *out; size_t out_size; int out_fd; + int hdr_valid; filter_std_hdr_t hdr; } filter_output_t; diff --git a/archive/pc_archive.c b/archive/pc_archive.c index 7da9608..be3680f 100644 --- a/archive/pc_archive.c +++ b/archive/pc_archive.c @@ -1028,6 +1028,7 @@ process_by_filter(int fd, int *typ, struct archive *target_arc, struct filter_info fi; int64_t wrtn; + fout->hdr_valid = 1; fi.source_arc = source_arc; fi.target_arc = target_arc; fi.entry = entry; @@ -1100,7 +1101,6 @@ copy_file_data(pc_ctx_t *pctx, struct archive *arc, struct archive_entry *entry, &fout, 1, pctx->level); if (rv != FILTER_RETURN_SKIP && rv != FILTER_RETURN_ERROR) { - pctx->ctype = TYPE_UNKNOWN; // Force analyzer on filter output if (fout.output_type == FILTER_OUTPUT_MEM) { archive_entry_xattr_add_entry(entry, FILTER_XATTR_ENTRY, fname, strlen(fname)); @@ -1108,10 +1108,12 @@ copy_file_data(pc_ctx_t *pctx, struct archive *arc, struct archive_entry *entry, close(fd); return (-1); } - rv = archive_write_data(arc, &(fout.hdr), - sizeof (fout.hdr)); - if (rv != sizeof (fout.hdr)) - return (rv); + if (fout.hdr_valid) { + rv = archive_write_data(arc, &(fout.hdr), + sizeof (fout.hdr)); + if (rv != sizeof (fout.hdr)) + return (rv); + } rv = archive_write_data(arc, fout.out, fout.out_size); free(fout.out); @@ -1177,7 +1179,6 @@ do_map: &fout, 1, pctx->level); if (rv != FILTER_RETURN_SKIP && rv != FILTER_RETURN_ERROR) { - pctx->ctype = TYPE_UNKNOWN; // Force analyzer on filter output if (fout.output_type == FILTER_OUTPUT_MEM) { archive_entry_xattr_add_entry(entry, FILTER_XATTR_ENTRY, @@ -1186,10 +1187,12 @@ do_map: close(fd); return (-1); } - rv = archive_write_data(arc, &(fout.hdr), - sizeof (fout.hdr)); - if (rv != sizeof (fout.hdr)) - return (rv); + if (fout.hdr_valid) { + rv = archive_write_data(arc, &(fout.hdr), + sizeof (fout.hdr)); + if (rv != sizeof (fout.hdr)) + return (rv); + } rv = archive_write_data(arc, fout.out, fout.out_size); free(fout.out); diff --git a/config b/config index e42d68f..8e74cce 100755 --- a/config +++ b/config @@ -262,6 +262,7 @@ then salsa20_stream_c='\$\(XSALSA20_STREAM_C\)' salsa20_stream_asm= salsa20_debug='\$\(XSALSA20_DEBUG\)' + extra_opt_flags="${extra_opt_flags} -fno-inline" else typ="RELEASE" fi diff --git a/filters/dispack/dis.cpp b/filters/dispack/dis.cpp index 113281b..f0f23c6 100644 --- a/filters/dispack/dis.cpp +++ b/filters/dispack/dis.cpp @@ -429,12 +429,17 @@ struct DisFilterCtx Buffer[i].ResetBuffer(); } - sInt DetectJumpTable(sU8 *instr,sU32 addr) + sInt DetectJumpTable(sU8 *instr, sU32 addr, sU8 *srcEnd) { assert(addr < CodeEnd); sInt nMax = (CodeEnd - addr) / 4; sInt count = 0; + // Check for overflow + if (instr + (nMax + 1) * 4 > srcEnd) { + return (0); + } + while(countexe_preprocess) { if (stype == TYPE_EXE32 || stype == TYPE_EXE64 || - stype == TYPE_ARCHIVE_AR) { + stype == TYPE_ARCHIVE_AR || stype == TYPE_EXE32_PE) { _dstlen = fromlen; memcpy(to, from, fromlen); if (Forward_E89(to, fromlen) == 0) { diff --git a/pcompress.h b/pcompress.h index 7391986..0c8410a 100644 --- a/pcompress.h +++ b/pcompress.h @@ -305,7 +305,7 @@ struct cmp_data { mac_ctx_t chunk_hmac; algo_props_t *props; int decompressing; - uchar_t btype; + int btype; pc_ctx_t *pctx; };