From 2c678757811579816d52eaef69df11145ed7c439 Mon Sep 17 00:00:00 2001 From: "Markus F.X.J. Oberhumer" Date: Thu, 26 Apr 2007 16:15:49 +0200 Subject: [PATCH] Pass and verify original data in all xxx_test_overlap() functions. --- src/compress.cpp | 12 +++++----- src/compress.h | 24 +++++++++++++------- src/compress_lzma.cpp | 51 +++++++++++++++++------------------------- src/compress_ucl.cpp | 7 ++++-- src/compress_zlib.cpp | 52 +++++++++++++++++-------------------------- src/conf.h | 6 +++-- src/p_djgpp2.cpp | 3 ++- src/p_djgpp2.h | 1 + src/p_tmt.cpp | 3 ++- src/p_tmt.h | 1 + src/p_unix.cpp | 8 +++++-- src/packer.cpp | 23 +++++++++++-------- src/packer.h | 2 ++ 13 files changed, 100 insertions(+), 93 deletions(-) diff --git a/src/compress.cpp b/src/compress.cpp index f0e40220..c3eba972 100644 --- a/src/compress.cpp +++ b/src/compress.cpp @@ -182,8 +182,10 @@ int upx_decompress ( const upx_bytep src, unsigned src_len, // **************************************************************************/ -int upx_test_overlap ( const upx_bytep buf, unsigned src_off, - unsigned src_len, unsigned* dst_len, +int upx_test_overlap ( const upx_bytep buf, + const upx_bytep tbuf, + unsigned src_off, unsigned src_len, + unsigned* dst_len, int method, const upx_compress_result_t *cresult ) { @@ -201,15 +203,15 @@ int upx_test_overlap ( const upx_bytep buf, unsigned src_off, } #if defined(WITH_LZMA) else if (M_IS_LZMA(method)) - r = upx_lzma_test_overlap(buf, src_off, src_len, dst_len, method, cresult); + r = upx_lzma_test_overlap(buf, tbuf, src_off, src_len, dst_len, method, cresult); #endif #if defined(WITH_NRV) else if (M_IS_NRV2B(method) || M_IS_NRV2D(method) || M_IS_NRV2E(method)) - r = upx_nrv_test_overlap(buf, src_off, src_len, dst_len, method, cresult); + r = upx_nrv_test_overlap(buf, tbuf, src_off, src_len, dst_len, method, cresult); #endif #if defined(WITH_UCL) else if (M_IS_NRV2B(method) || M_IS_NRV2D(method) || M_IS_NRV2E(method)) - r = upx_ucl_test_overlap(buf, src_off, src_len, dst_len, method, cresult); + r = upx_ucl_test_overlap(buf, tbuf, src_off, src_len, dst_len, method, cresult); #endif else { throwInternalError("unknown decompression method"); diff --git a/src/compress.h b/src/compress.h index 7e7b95e8..df10aab6 100644 --- a/src/compress.h +++ b/src/compress.h @@ -46,8 +46,10 @@ int upx_lzma_decompress ( const upx_bytep src, unsigned src_len, upx_bytep dst, unsigned* dst_len, int method, const upx_compress_result_t *cresult ); -int upx_lzma_test_overlap ( const upx_bytep buf, unsigned src_off, - unsigned src_len, unsigned* dst_len, +int upx_lzma_test_overlap ( const upx_bytep buf, + const upx_bytep tbuf, + unsigned src_off, unsigned src_len, + unsigned* dst_len, int method, const upx_compress_result_t *cresult ); #endif @@ -65,8 +67,10 @@ int upx_nrv_decompress ( const upx_bytep src, unsigned src_len, upx_bytep dst, unsigned* dst_len, int method, const upx_compress_result_t *cresult ); -int upx_nrv_test_overlap ( const upx_bytep buf, unsigned src_off, - unsigned src_len, unsigned* dst_len, +int upx_nrv_test_overlap ( const upx_bytep buf, + const upx_bytep tbuf, + unsigned src_off, unsigned src_len, + unsigned* dst_len, int method, const upx_compress_result_t *cresult ); #endif @@ -84,8 +88,10 @@ int upx_ucl_decompress ( const upx_bytep src, unsigned src_len, upx_bytep dst, unsigned* dst_len, int method, const upx_compress_result_t *cresult ); -int upx_ucl_test_overlap ( const upx_bytep buf, unsigned src_off, - unsigned src_len, unsigned* dst_len, +int upx_ucl_test_overlap ( const upx_bytep buf, + const upx_bytep tbuf, + unsigned src_off, unsigned src_len, + unsigned* dst_len, int method, const upx_compress_result_t *cresult ); #endif @@ -102,8 +108,10 @@ int upx_zlib_decompress ( const upx_bytep src, unsigned src_len, upx_bytep dst, unsigned* dst_len, int method, const upx_compress_result_t *cresult ); -int upx_zlib_test_overlap ( const upx_bytep buf, unsigned src_off, - unsigned src_len, unsigned* dst_len, +int upx_zlib_test_overlap ( const upx_bytep buf, + const upx_bytep tbuf, + unsigned src_off, unsigned src_len, + unsigned* dst_len, int method, const upx_compress_result_t *cresult ); #endif diff --git a/src/compress_lzma.cpp b/src/compress_lzma.cpp index f0cabc7a..261a0f66 100644 --- a/src/compress_lzma.cpp +++ b/src/compress_lzma.cpp @@ -28,6 +28,7 @@ #include "conf.h" #include "compress.h" +#include "mem.h" void lzma_compress_config_t::reset() @@ -483,44 +484,32 @@ error: /************************************************************************* -// test_overlap +// test_overlap - see for semantics **************************************************************************/ -// from : -// test an overlapping in-place decompression within a buffer: -// - try a virtual decompression from &buf[src_off] -> &buf[0] -// - no data is actually written -// - only the bytes at buf[src_off..src_off+src_len-1] will get accessed -// -// 2007-04-25 However, I do not see any "virtual decompress" function in lzma -// that avoids writing the result. Therefore, do an actual decompress. - -int upx_lzma_test_overlap ( const upx_bytep buf, unsigned src_off, - unsigned src_len, unsigned* dst_len, +int upx_lzma_test_overlap ( const upx_bytep buf, + const upx_bytep tbuf, + unsigned src_off, unsigned src_len, + unsigned* dst_len, int method, const upx_compress_result_t *cresult ) { assert(M_IS_LZMA(method)); - // Note that Packer::verifyOverlappingDecompression() will - // verify the final result in any case. - unsigned dlen = *dst_len; - unsigned const overlap_overhead = src_off + src_len - dlen; - // printf("upx_lzma_test_overlap: %d\n", overlap_overhead); - - upx_bytep const dst = (upx_bytep)malloc(src_off + src_len); - if (dst) { - upx_bytep const src = &dst[src_off]; - // High ends of src and dst are equal (including overlap_overhead.) - memcpy(src, &buf[src_off], src_len); - int const rv = upx_lzma_decompress(src, src_len, dst, &dlen, - method, cresult); - free(dst); - if (UPX_E_OK==rv) { - return UPX_E_OK; - } - } - return UPX_E_ERROR; + MemBuffer b(src_off + src_len); + memcpy(b + src_off, buf + src_off, src_len); + unsigned saved_dst_len = *dst_len; + int r = upx_lzma_decompress(b + src_off, src_len, b, dst_len, method, cresult); + if (r != UPX_E_OK) + return r; + if (*dst_len != saved_dst_len) + return UPX_E_ERROR; + // NOTE: there is a very tiny possibility that decompression has + // succeeded but the data is not restored correctly because of + // in-place buffer overlapping. + if (tbuf != NULL && memcmp(tbuf, b, *dst_len) != 0) + return UPX_E_ERROR; + return UPX_E_OK; } diff --git a/src/compress_ucl.cpp b/src/compress_ucl.cpp index 8d00f179..c314458f 100644 --- a/src/compress_ucl.cpp +++ b/src/compress_ucl.cpp @@ -228,12 +228,15 @@ int upx_ucl_decompress ( const upx_bytep src, unsigned src_len, // **************************************************************************/ -int upx_ucl_test_overlap ( const upx_bytep buf, unsigned src_off, - unsigned src_len, unsigned* dst_len, +int upx_ucl_test_overlap ( const upx_bytep buf, + const upx_bytep tbuf, + unsigned src_off, unsigned src_len, + unsigned* dst_len, int method, const upx_compress_result_t *cresult ) { int r; + UNUSED(tbuf); // not needed for UCL switch (method) { diff --git a/src/compress_zlib.cpp b/src/compress_zlib.cpp index 29b0d516..9b9840dc 100644 --- a/src/compress_zlib.cpp +++ b/src/compress_zlib.cpp @@ -28,6 +28,7 @@ #include "conf.h" #include "compress.h" +#include "mem.h" void zlib_compress_config_t::reset() @@ -188,45 +189,32 @@ done: /************************************************************************* -// test_overlap +// test_overlap - see for semantics **************************************************************************/ -// from : -// test an overlapping in-place decompression within a buffer: -// - try a virtual decompression from &buf[src_off] -> &buf[0] -// - no data is actually written -// - only the bytes at buf[src_off..src_off+src_len-1] will get accessed -// -// 2007-04-25 However, I do not see any "virtual decompress" function in zlib -// that avoids writing the result. Therefore, do an actual decompress. - -int upx_zlib_test_overlap ( const upx_bytep buf, unsigned src_off, - unsigned src_len, unsigned* dst_len, +int upx_zlib_test_overlap ( const upx_bytep buf, + const upx_bytep tbuf, + unsigned src_off, unsigned src_len, + unsigned* dst_len, int method, const upx_compress_result_t *cresult ) { assert(method == M_DEFLATE); - // Note that Packer::verifyOverlappingDecompression() will - // verify the final result in any case. - - unsigned dlen = *dst_len; - //unsigned overlap_overhead = src_off + src_len - dlen; - //printf("upx_zlib_test_overlap: %d\n", overlap_overhead); - - upx_bytep const dst = (upx_bytep)malloc(src_off + src_len); - if (dst) { - upx_bytep const src = &dst[src_off]; - // High ends of src and dst are equal (including overlap_overhead.) - memcpy(src, &buf[src_off], src_len); - int const rv = upx_zlib_decompress(src, src_len, dst, &dlen, - method, cresult); - free(dst); - if (UPX_E_OK==rv) { - return UPX_E_OK; - } - } - return UPX_E_ERROR; + MemBuffer b(src_off + src_len); + memcpy(b + src_off, buf + src_off, src_len); + unsigned saved_dst_len = *dst_len; + int r = upx_zlib_decompress(b + src_off, src_len, b, dst_len, method, cresult); + if (r != UPX_E_OK) + return r; + if (*dst_len != saved_dst_len) + return UPX_E_ERROR; + // NOTE: there is a very tiny possibility that decompression has + // succeeded but the data is not restored correctly because of + // in-place buffer overlapping. + if (tbuf != NULL && memcmp(tbuf, b, *dst_len) != 0) + return UPX_E_ERROR; + return UPX_E_OK; } diff --git a/src/conf.h b/src/conf.h index 7b9620f2..6ed1347b 100644 --- a/src/conf.h +++ b/src/conf.h @@ -779,8 +779,10 @@ int upx_decompress ( const upx_bytep src, unsigned src_len, upx_bytep dst, unsigned* dst_len, int method, const upx_compress_result_t *cresult ); -int upx_test_overlap ( const upx_bytep buf, unsigned src_off, - unsigned src_len, unsigned* dst_len, +int upx_test_overlap ( const upx_bytep buf, + const upx_bytep tbuf, + unsigned src_off, unsigned src_len, + unsigned* dst_len, int method, const upx_compress_result_t *cresult ); diff --git a/src/p_djgpp2.cpp b/src/p_djgpp2.cpp index c3625f82..b6243431 100644 --- a/src/p_djgpp2.cpp +++ b/src/p_djgpp2.cpp @@ -76,10 +76,11 @@ const int *PackDjgpp2::getFilters() const unsigned PackDjgpp2::findOverlapOverhead(const upx_bytep buf, + const upx_bytep tbuf, unsigned range, unsigned upper_limit) const { - unsigned o = super::findOverlapOverhead(buf, range, upper_limit); + unsigned o = super::findOverlapOverhead(buf, tbuf, range, upper_limit); o = (o + 0x3ff) &~ 0x1ff; return o; } diff --git a/src/p_djgpp2.h b/src/p_djgpp2.h index adc5d504..30952ae6 100644 --- a/src/p_djgpp2.h +++ b/src/p_djgpp2.h @@ -58,6 +58,7 @@ protected: virtual int readFileHeader(); virtual unsigned findOverlapOverhead(const upx_bytep buf, + const upx_bytep tbuf, unsigned range = 0, unsigned upper_limit = ~0u) const; virtual void buildLoader(const Filter *ft); diff --git a/src/p_tmt.cpp b/src/p_tmt.cpp index 58cf34d5..40c83dcb 100644 --- a/src/p_tmt.cpp +++ b/src/p_tmt.cpp @@ -67,11 +67,12 @@ const int *PackTmt::getFilters() const unsigned PackTmt::findOverlapOverhead(const upx_bytep buf, + const upx_bytep tbuf, unsigned range, unsigned upper_limit) const { // make sure the decompressor will be paragraph aligned - unsigned o = super::findOverlapOverhead(buf, range, upper_limit); + unsigned o = super::findOverlapOverhead(buf, tbuf, range, upper_limit); o = ((o + 0x20) &~ 0xf) - (ph.u_len & 0xf); return o; } diff --git a/src/p_tmt.h b/src/p_tmt.h index 59d1a2a2..eb469a9a 100644 --- a/src/p_tmt.h +++ b/src/p_tmt.h @@ -56,6 +56,7 @@ protected: virtual int readFileHeader(); virtual unsigned findOverlapOverhead(const upx_bytep buf, + const upx_bytep tbuf, unsigned range = 0, unsigned upper_limit = ~0u) const; virtual void buildLoader(const Filter *ft); diff --git a/src/p_unix.cpp b/src/p_unix.cpp index aca648de..5e32985b 100644 --- a/src/p_unix.cpp +++ b/src/p_unix.cpp @@ -173,8 +173,10 @@ void PackUnix::pack2(OutputFile *fo, Filter &ft) compressWithFilters(&ft, OVERHEAD, NULL_cconf, filter_strategy); if (ph.c_len < ph.u_len) { + const upx_bytep tbuf = NULL; + if (ft.id == 0) tbuf = ibuf; ph.overlap_overhead = OVERHEAD; - if (!testOverlappingDecompression(obuf, ph.overlap_overhead)) { + if (!testOverlappingDecompression(obuf, tbuf, ph.overlap_overhead)) { // not in-place compressible ph.c_len = ph.u_len; } @@ -353,8 +355,10 @@ void PackUnix::packExtent( } if (ph.c_len < ph.u_len) { + const upx_bytep tbuf = NULL; + if (ft == NULL || ft->id == 0) tbuf = ibuf; ph.overlap_overhead = OVERHEAD; - if (!testOverlappingDecompression(obuf, ph.overlap_overhead)) { + if (!testOverlappingDecompression(obuf, tbuf, ph.overlap_overhead)) { // not in-place compressible ph.c_len = ph.u_len; } diff --git a/src/packer.cpp b/src/packer.cpp index 1199854f..68f22aa0 100644 --- a/src/packer.cpp +++ b/src/packer.cpp @@ -386,13 +386,16 @@ void Packer::decompress(const upx_bytep in, upx_bytep out, // overlapping decompression **************************************************************************/ -bool ph_testOverlappingDecompression(const PackHeader &ph, const upx_bytep buf, +bool ph_testOverlappingDecompression(const PackHeader &ph, + const upx_bytep buf, + const upx_bytep tbuf, unsigned overlap_overhead) { if (ph.c_len >= ph.u_len) return false; - assert((int)overlap_overhead >= 0); + assert((int) overlap_overhead >= 0); + assert((int) (ph.u_len + overlap_overhead) >= 0); // Because upx_test_overlap() does not use the asm_fast decompressor // we must account for extra 3 bytes that asm_fast does use, @@ -406,16 +409,17 @@ bool ph_testOverlappingDecompression(const PackHeader &ph, const upx_bytep buf, unsigned src_off = ph.u_len + overlap_overhead - ph.c_len; unsigned new_len = ph.u_len; - int r = upx_test_overlap(buf - src_off, src_off, - ph.c_len, &new_len, ph.method, &ph.compress_result); + int r = upx_test_overlap(buf - src_off, tbuf, + src_off, ph.c_len, &new_len, + ph.method, &ph.compress_result); return (r == UPX_E_OK && new_len == ph.u_len); } -bool Packer::testOverlappingDecompression(const upx_bytep buf, +bool Packer::testOverlappingDecompression(const upx_bytep buf, const upx_bytep tbuf, unsigned overlap_overhead) const { - return ph_testOverlappingDecompression(ph, buf, overlap_overhead); + return ph_testOverlappingDecompression(ph, buf, tbuf, overlap_overhead); } @@ -474,6 +478,7 @@ void Packer::verifyOverlappingDecompression(upx_bytep o_ptr, unsigned o_size, Fi **************************************************************************/ unsigned Packer::findOverlapOverhead(const upx_bytep buf, + const upx_bytep tbuf, unsigned range, unsigned upper_limit) const { @@ -493,8 +498,8 @@ unsigned Packer::findOverlapOverhead(const upx_bytep buf, assert(m >= low); assert(m <= high); assert(m < overhead || overhead == 0); nr++; - bool success = testOverlappingDecompression(buf, m); - //printf("testOverlapOverhead: %d %d -> %d\n", nr, m, (int)success); + bool success = testOverlappingDecompression(buf, tbuf, m); + printf("testOverlapOverhead(%d): %d %d: %d -> %d\n", nr, low, high, m, (int)success); if (success) { overhead = m; @@ -1413,7 +1418,7 @@ void Packer::compressWithFilters(upx_bytep i_ptr, unsigned i_len, if (ph.c_len + lsize + hdr_c_len <= best_ph.c_len + best_ph_lsize + best_hdr_c_len) { // get results - ph.overlap_overhead = findOverlapOverhead(o_tmp, overlap_range); + ph.overlap_overhead = findOverlapOverhead(o_tmp, i_ptr, overlap_range); buildLoader(&ft); lsize = getLoaderSize(); assert(lsize > 0); diff --git a/src/packer.h b/src/packer.h index ac165e92..e11bd32b 100644 --- a/src/packer.h +++ b/src/packer.h @@ -203,9 +203,11 @@ protected: // util for verifying overlapping decompresion // non-destructive test virtual bool testOverlappingDecompression(const upx_bytep buf, + const upx_bytep tbuf, unsigned overlap_overhead) const; // non-destructive find virtual unsigned findOverlapOverhead(const upx_bytep buf, + const upx_bytep tbuf, unsigned range = 0, unsigned upper_limit = ~0u) const; // destructive decompress + verify