From d335d7bafd6a97bc39bf2d70c60e07abd8fc0874 Mon Sep 17 00:00:00 2001 From: "Markus F.X.J. Oberhumer" Date: Thu, 6 Oct 2016 09:51:30 +0200 Subject: [PATCH 1/5] p_mach.h: fix C++ syntax. --- src/p_mach.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/p_mach.h b/src/p_mach.h index 91b093cb..40ec3f33 100644 --- a/src/p_mach.h +++ b/src/p_mach.h @@ -974,7 +974,7 @@ protected: virtual void addStubEntrySections(Filter const *); __packed_struct(Mach_thread_command) - typedef typename MachITypes::Word Word; + typedef MachITypes::Word Word; Word cmd; /* LC_THREAD or LC_UNIXTHREAD */ Word cmdsize; /* total size of this command */ Word flavor; From 8f5e89c900b3e94af012f5da2f4627a5fff52298 Mon Sep 17 00:00:00 2001 From: "Markus F.X.J. Oberhumer" Date: Thu, 6 Oct 2016 01:26:01 +0200 Subject: [PATCH 2/5] Update testsuite. --- .github/travis_testsuite_1.sh | 49 +++++++++++++++++++++++++++---- src/Makefile | 47 +++++++++++++++-------------- src/stub/scripts/upx-clang-format | 10 ++++--- 3 files changed, 75 insertions(+), 31 deletions(-) diff --git a/.github/travis_testsuite_1.sh b/.github/travis_testsuite_1.sh index 26d84a28..81c0966b 100644 --- a/.github/travis_testsuite_1.sh +++ b/.github/travis_testsuite_1.sh @@ -55,26 +55,41 @@ testsuite_check_sha() { if ! cmp -s $1/.sha256sums.expected $1/.sha256sums.current; then echo "UPX-ERROR: checksum mismatch" diff -u $1/.sha256sums.expected $1/.sha256sums.current || true - #exit 1 exit_code=1 let num_errors+=1 || true + #exit 1 fi echo } +testsuite_check_sha_decompressed() { + (cd "$1" && sha256sum -b */* | LC_ALL=C sort -k2) > $1/.sha256sums.current + if ! cmp -s $1/.sha256sums.expected $1/.sha256sums.current; then + cat $1/.sha256sums.current + echo "UPX-ERROR: decompressed checksum mismatch" + diff -u $1/.sha256sums.expected $1/.sha256sums.current || true + exit 1 + fi +} + testsuite_run_compress() { testsuite_header $testdir local f - for f in t01_decompressed/*/*; do + for f in t01_canonicalized/*/*; do testsuite_split_f $f [[ -z $fb ]] && continue - mkdir -p $testdir/$fsubdir + mkdir -p $testdir/$fsubdir $testdir/.decompressed/$fsubdir $upx_run --prefer-ucl "$@" $f -o $testdir/$fsubdir/$fb + $upx_run -qq -d $testdir/$fsubdir/$fb -o $testdir/.decompressed/$fsubdir/$fb done testsuite_check_sha $testdir $upx_run -l $testdir/*/* $upx_run --file-info $testdir/*/* $upx_run -t $testdir/*/* + # check that after decompression the file matches the canonicalized version + cp t01_canonicalized/.sha256sums.expected $testdir/.decompressed/ + testsuite_check_sha_decompressed $testdir/.decompressed + rm -rf ./$testdir/.decompressed } # /*********************************************************************** @@ -116,8 +131,7 @@ if [[ $BM_B =~ (^|\+)coverage($|\+) ]]; then (cd / && cd $upx_BUILDDIR && lcov -d . --zerocounters) fi -export UPX= -export UPX="--no-color --no-progress" +export UPX="--prefer-ucl --no-color --no-progress" # let's go if ! $upx_run --version >/dev/null; then exit 1; fi @@ -153,6 +167,31 @@ done testsuite_check_sha $testdir +# run one pack+unpack step to canonicalize the files +testdir=t01_canonicalized +mkdir $testdir; echo -n "\ +24158f78c34c4ef94bb7773a6dda7231d289be76c2f5f60e8b9ddb3f800c100e *amd64-linux.elf/upx-3.91 +28d7ca8f0dfca8159e637eaf2057230b6e6719e07751aca1d19a45b5efed817c *arm-wince.pe/upx-3.91.exe +b1c1c38d50007616aaf8e942839648c80a6111023e0b411e5fa7a06c543aeb4a *armeb-linux.elf/upx-3.91 +bcac77a287289301a45fde9a75e4e6c9ad7f8d57856bae6eafaae12ae4445a34 *i386-dos32.djgpp2.coff/upx-3.91.exe +730a513b72a094697f827e4ac1a4f8ef58a614fc7a7ad448fa58d60cd89af7ed *i386-linux.elf/upx-3.91 +1f378dc3f8c6f0cf604c057b468c22beb46a16837bd2410cf5f8fa44a42b2f6f *i386-win32.pe/upx-3.91.exe +8e5333ea040f5594d3e67d5b09e005d52b3a52ef55099a7c11d7e39ead38e66d *m68k-atari.tos/upx-3.91.ttp +c3f44b4d00a87384c03a6f9e7aec809c1addfe3e271244d38a474f296603088c *mipsel-linux.elf/upx-3.91 +b8c35fa2956da17ca505956e9f5017bb5f3a746322647e24ccb8ff28059cafa4 *powerpc-linux.elf/upx-3.91 +" > $testdir/.sha256sums.expected + +testsuite_header $testdir +for f in t01_decompressed/*/*; do + testsuite_split_f $f + [[ -z $fb ]] && continue + mkdir -p $testdir/$fsubdir/.packed + $upx_run --prefer-ucl -1 $f -o $testdir/$fsubdir/.packed/$fb + $upx_run -d $testdir/$fsubdir/.packed/$fb -o $testdir/$fsubdir/$fb +done +testsuite_check_sha $testdir + + # /*********************************************************************** # // compression tests # // info: we use fast compression levels because we want to diff --git a/src/Makefile b/src/Makefile index 0d15dac9..dfac2b0d 100644 --- a/src/Makefile +++ b/src/Makefile @@ -141,6 +141,31 @@ help$(objext): $(MAKEFILE_LIST) endif +# "make run-testsuite" +# +# search for the UPX testsuite -- git clone https://github.com/upx/upx-testsuite.git +# you also can set upx_testsuite_SRCDIR +ifndef upx_testsuite_SRCDIR +# search standard locations below $(top_srcdir) +ifneq ($(wildcard $(top_srcdir)/../upx-testsuite.git/files/packed/.),) +upx_testsuite_SRCDIR := $(top_srcdir)/../upx-testsuite.git +else ifneq ($(wildcard $(top_srcdir)/../upx-testsuite/files/packed/.),) +upx_testsuite_SRCDIR := $(top_srcdir)/../upx-testsuite +endif +endif +# run the UPX testsuite +ifneq ($(wildcard $(upx_testsuite_SRCDIR)/files/packed/.),) +ifneq ($(wildcard $(top_srcdir)/.github/travis_testsuite_1.sh),) +run-testsuite: export upx_exe := ./upx$(exeext) +run-testsuite: export upx_testsuite_SRCDIR := $(upx_testsuite_SRCDIR) +run-testsuite: export upx_testsuite_BUILDDIR := ./tmp-testsuite +run-testsuite: ./upx$(exeext) + time -p bash $(top_srcdir)/.github/travis_testsuite_1.sh +.PHONY: run-testsuite +endif +endif + + # automatically format some C++ source code files ifeq ($(shell uname),Linux) CLANG_FORMAT_FILES += packhead.cpp @@ -153,26 +178,4 @@ clang-format: .PHONY: clang-format endif - -# run the UPX testsuite - git clone https://github.com/upx/upx-testsuite.git -# you have to set upx_testsuite_SRCDIR -ifneq ($(wildcard $(top_srcdir)/.github/travis_testsuite_1.sh),) -ifndef upx_testsuite_SRCDIR -# search standard locations below $(top_srcdir) -ifneq ($(wildcard $(top_srcdir)/../upx-testsuite.git/files/packed/.),) -upx_testsuite_SRCDIR := $(top_srcdir)/../upx-testsuite.git -else ifneq ($(wildcard $(top_srcdir)/../upx-testsuite/files/packed/.),) -upx_testsuite_SRCDIR := $(top_srcdir)/../upx-testsuite -endif -endif -ifneq ($(wildcard $(upx_testsuite_SRCDIR)/files/packed/.),) -run-testsuite: export upx_exe := ./upx$(exeext) -run-testsuite: export upx_testsuite_SRCDIR := $(upx_testsuite_SRCDIR) -run-testsuite: export upx_testsuite_BUILDDIR := ./tmp-testsuite -run-testsuite: ./upx$(exeext) - time -p bash $(top_srcdir)/.github/travis_testsuite_1.sh -.PHONY: run-testsuite -endif -endif - # vim:set ts=8 sw=8 noet: diff --git a/src/stub/scripts/upx-clang-format b/src/stub/scripts/upx-clang-format index 422f2e66..9833f661 100755 --- a/src/stub/scripts/upx-clang-format +++ b/src/stub/scripts/upx-clang-format @@ -1,15 +1,17 @@ -#!/bin/sh -set -e +#! /usr/bin/env bash +## vim:set ts=4 sw=4 et: +set -e; set -o pipefail # NOTE: we are using clang-format-3.9.0 from upx-stubtools # see https://github.com/upx/upx-stubtools/releases CLANG_FORMAT="$HOME/local/bin/bin-upx/clang-format-3.9.0" -if ! test -f "$CLANG_FORMAT"; then +if [[ ! -f $CLANG_FORMAT ]]; then CLANG_FORMAT="$HOME/bin/bin-upx/clang-format-3.9.0" fi -if ! test -f "$CLANG_FORMAT"; then +if [[ ! -f $CLANG_FORMAT ]]; then echo "ERROR: $0: cannot find clang-format-3.9.0" + echo "ERROR: $0: please visit https://github.com/upx/upx-stubtools/releases" exit 1 fi From 022ba32c1ab58ff6935c8f147f5cfe247cb5402d Mon Sep 17 00:00:00 2001 From: "Markus F.X.J. Oberhumer" Date: Thu, 6 Oct 2016 12:11:32 +0200 Subject: [PATCH 3/5] Improve robustness of seek() by adding some sanity checks. --- src/file.cpp | 12 ++++++++---- src/file.h | 6 +++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/file.cpp b/src/file.cpp index c2752624..b6042017 100644 --- a/src/file.cpp +++ b/src/file.cpp @@ -168,8 +168,10 @@ void FileBase::write(const void *buf, int len) } -off_t FileBase::seek(off_t off, int whence) +off_t FileBase::seek(upx_int64_t off64, int whence) { + (void) mem_size(1, off64 >= 0 ? off64 : -off64); // sanity check + off_t off = ACC_ICONV(off_t, off64); if (!isOpen()) throwIOException("bad seek 1"); if (whence == SEEK_SET) { @@ -285,9 +287,9 @@ int InputFile::readx(MemBuffer &buf, int len) } -off_t InputFile::seek(off_t off, int whence) +off_t InputFile::seek(upx_int64_t off64, int whence) { - off_t pos = super::seek(off,whence); + off_t pos = super::seek(off64, whence); if (_length < pos) throwIOException("bad seek 4"); return pos; @@ -402,8 +404,10 @@ void OutputFile::rewrite(const void *buf, int len) bytes_written -= len; // restore } -off_t OutputFile::seek(off_t off, int whence) +off_t OutputFile::seek(upx_int64_t off64, int whence) { + (void) mem_size(1, off64 >= 0 ? off64 : -off64); // sanity check + off_t off = ACC_ICONV(off_t, off64); assert(!opt->to_stdout); switch (whence) { case SEEK_SET: { diff --git a/src/file.h b/src/file.h index d895efd6..0ddd84a6 100644 --- a/src/file.h +++ b/src/file.h @@ -67,7 +67,7 @@ protected: virtual int read(void *buf, int len); virtual int readx(void *buf, int len); virtual void write(const void *buf, int len); - virtual off_t seek(off_t off, int whence); + virtual off_t seek(upx_int64_t off, int whence); virtual off_t tell() const; int _fd; @@ -106,7 +106,7 @@ public: virtual int read(MemBuffer &buf, int len); virtual int readx(MemBuffer &buf, int len); - virtual off_t seek(off_t off, int whence); + virtual off_t seek(upx_int64_t off, int whence); virtual off_t tell() const; virtual off_t st_size_orig() const; protected: @@ -142,7 +142,7 @@ public: virtual off_t st_size() const; // { return _length; } // FIXME - these won't work when using the '--stdout' option - virtual off_t seek(off_t off, int whence); + virtual off_t seek(upx_int64_t off, int whence); virtual void rewrite(const void *buf, int len); // util From b3a8d02caf883ceb412a0c50047fce29b386a37c Mon Sep 17 00:00:00 2001 From: "Markus F.X.J. Oberhumer" Date: Thu, 6 Oct 2016 12:31:03 +0200 Subject: [PATCH 4/5] Cosmetic cleanups. --- src/conf.h | 2 +- src/file.cpp | 4 ++-- src/mem.cpp | 32 ++++++++++++++++---------------- src/util.h | 4 ++++ 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/conf.h b/src/conf.h index c5f82113..9d8befda 100644 --- a/src/conf.h +++ b/src/conf.h @@ -472,7 +472,7 @@ struct upx_callback_t template struct OptVar { - typedef T Type; + typedef T value_type; static const T default_value_c = default_value; static const T min_value_c = min_value; static const T max_value_c = max_value; diff --git a/src/file.cpp b/src/file.cpp index b6042017..0f93956e 100644 --- a/src/file.cpp +++ b/src/file.cpp @@ -170,7 +170,7 @@ void FileBase::write(const void *buf, int len) off_t FileBase::seek(upx_int64_t off64, int whence) { - (void) mem_size(1, off64 >= 0 ? off64 : -off64); // sanity check + mem_size_assert(1, off64 >= 0 ? off64 : -off64); // sanity check off_t off = ACC_ICONV(off_t, off64); if (!isOpen()) throwIOException("bad seek 1"); @@ -406,7 +406,7 @@ void OutputFile::rewrite(const void *buf, int len) off_t OutputFile::seek(upx_int64_t off64, int whence) { - (void) mem_size(1, off64 >= 0 ? off64 : -off64); // sanity check + mem_size_assert(1, off64 >= 0 ? off64 : -off64); // sanity check off_t off = ACC_ICONV(off_t, off64); assert(!opt->to_stdout); switch (whence) { diff --git a/src/mem.cpp b/src/mem.cpp index 15f5af15..84721e2f 100644 --- a/src/mem.cpp +++ b/src/mem.cpp @@ -52,7 +52,7 @@ size_t mem_size(upx_uint64_t element_size, upx_uint64_t n, upx_uint64_t extra) size_t mem_size_get_n(upx_uint64_t element_size, upx_uint64_t n) { - (void) mem_size(element_size, n); // check + mem_size_assert(element_size, n); return ACC_ICONV(size_t, n); // return n } @@ -90,29 +90,29 @@ int ptr_diff(const char *p1, const char *p2) /************************************************************************* -// bool use_mcheck() +// bool use_simple_mcheck() **************************************************************************/ #if defined(__SANITIZE_ADDRESS__) -__acc_static_forceinline bool use_mcheck() { return false; } +__acc_static_forceinline bool use_simple_mcheck() { return false; } #elif (WITH_VALGRIND) && defined(RUNNING_ON_VALGRIND) -static int use_mcheck_flag = -1; -__acc_static_noinline void use_mcheck_init() +static int use_simple_mcheck_flag = -1; +__acc_static_noinline void use_simple_mcheck_init() { - use_mcheck_flag = 1; + use_simple_mcheck_flag = 1; if (RUNNING_ON_VALGRIND) { - use_mcheck_flag = 0; + use_simple_mcheck_flag = 0; //fprintf(stderr, "upx: detected RUNNING_ON_VALGRIND\n"); } } -__acc_static_forceinline bool use_mcheck() +__acc_static_forceinline bool use_simple_mcheck() { - if __acc_unlikely(use_mcheck_flag < 0) - use_mcheck_init(); - return (bool) use_mcheck_flag; + if __acc_unlikely(use_simple_mcheck_flag < 0) + use_simple_mcheck_init(); + return (bool) use_simple_mcheck_flag; } #else -__acc_static_forceinline bool use_mcheck() { return true; } +__acc_static_forceinline bool use_simple_mcheck() { return true; } #endif @@ -137,7 +137,7 @@ void MemBuffer::dealloc() if (b != NULL) { checkState(); - if (use_mcheck()) + if (use_simple_mcheck()) { // remove magic constants set_be32(b - 8, 0); @@ -217,7 +217,7 @@ void MemBuffer::checkState() const { if (!b) throwInternalError("block not allocated"); - if (use_mcheck()) + if (use_simple_mcheck()) { if (get_be32(b - 4) != MAGIC1(b)) throwInternalError("memory clobbered before allocated block 1"); @@ -237,12 +237,12 @@ void MemBuffer::alloc(upx_uint64_t size) assert(b_size == 0); // assert(size > 0); - size_t bytes = mem_size(1, size, use_mcheck() ? 32 : 0); + size_t bytes = mem_size(1, size, use_simple_mcheck() ? 32 : 0); unsigned char *p = (unsigned char *) malloc(bytes); if (!p) throwOutOfMemoryException(); b_size = ACC_ICONV(unsigned, size); - if (use_mcheck()) + if (use_simple_mcheck()) { b = p + 16; // store magic constants to detect buffer overruns diff --git a/src/util.h b/src/util.h index 648f3b49..9d8aff74 100644 --- a/src/util.h +++ b/src/util.h @@ -63,6 +63,10 @@ int mem_replace(void *b, int blen, const void *what, int wlen, const void *r); size_t mem_size(upx_uint64_t element_size, upx_uint64_t n, upx_uint64_t extra = 0); size_t mem_size_get_n(upx_uint64_t element_size, upx_uint64_t n); +inline void mem_size_assert(upx_uint64_t element_size, upx_uint64_t n, upx_uint64_t extra = 0) { + (void) mem_size(element_size, n, extra); // sanity check +} + bool mem_size_valid(upx_uint64_t element_size, upx_uint64_t n, upx_uint64_t extra = 0); bool mem_size_valid_bytes(upx_uint64_t bytes); From 6e76f8ef3a1a11c1dd1793dba988057a0a9c7685 Mon Sep 17 00:00:00 2001 From: "Markus F.X.J. Oberhumer" Date: Thu, 6 Oct 2016 13:04:46 +0200 Subject: [PATCH 5/5] Don't use variable length arrays (VLA). --- src/Makefile | 2 +- src/p_mach.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Makefile b/src/Makefile index dfac2b0d..1879c686 100644 --- a/src/Makefile +++ b/src/Makefile @@ -68,7 +68,7 @@ CXXFLAGS += -fno-delete-null-pointer-checks endif CXXFLAGS += -fno-strict-aliasing -fwrapv CXXFLAGS += -funsigned-char -CXXFLAGS += -Wall -W -Wcast-align -Wcast-qual -Wmissing-declarations -Wpointer-arith -Wshadow -Wwrite-strings +CXXFLAGS += -Wall -W -Wcast-align -Wcast-qual -Wmissing-declarations -Wpointer-arith -Wshadow -Wvla -Wwrite-strings CXXFLAGS_WERROR ?= -Werror CXXFLAGS += $(CXXFLAGS_WERROR) diff --git a/src/p_mach.cpp b/src/p_mach.cpp index 0b3d38bb..add30006 100644 --- a/src/p_mach.cpp +++ b/src/p_mach.cpp @@ -684,8 +684,8 @@ void PackMachBase::pack4(OutputFile *fo, Filter &ft) // append PackHeader } if (my_filetype == Mach_header::MH_EXECUTE) { // Get a writeable copy of the stub to make editing easier. - unsigned char upxstub[sz_stub_main]; - memcpy(upxstub, stub_main, sizeof(upxstub)); + ByteArray(upxstub, sz_stub_main); + memcpy(upxstub, stub_main, sz_stub_main); Mach_header *const mhp = (Mach_header *)upxstub; char *tail = (char *)(1+ mhp);