From 25d96f1e4d84b9fca8f754cd91b822fc32758e5c Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 17 Apr 2019 15:01:53 -0700 Subject: fix out-of-bound read within LZ4_decompress_fast() and deprecate LZ4_decompress_fast(), with deprecation warnings enabled by default. Note that, as a consequence of the fix, LZ4_decompress_fast is now __slower__ than LZ4_decompress_safe(). That's because, since it doesn't know the input buffer size, it must progress more cautiously into the input buffer to ensure to out-of-bound read. --- lib/lz4.c | 15 +++++++++++++-- lib/lz4.h | 19 ++++++++++--------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/lib/lz4.c b/lib/lz4.c index bd8fa11..2f8aa04 100644 --- a/lib/lz4.c +++ b/lib/lz4.c @@ -135,6 +135,9 @@ # endif /* _MSC_VER */ #endif /* LZ4_FORCE_INLINE */ +#undef LZ4_FORCE_INLINE +#define LZ4_FORCE_INLINE static /* disable */ + /* LZ4_FORCE_O2_GCC_PPC64LE and LZ4_FORCE_O2_INLINE_GCC_PPC64LE * Gcc on ppc64le generates an unrolled SIMDized loop for LZ4_wildCopy, * together with a simple 8-byte copy loop as a fall-back path. @@ -1671,7 +1674,10 @@ LZ4_decompress_generic( { goto safe_literal_copy; } - LZ4_wildCopy32(op, ip, cpy); + if (endOnInput) + LZ4_wildCopy32(op, ip, cpy); + else + LZ4_wildCopy(op, ip, cpy); /* LZ4_decompress_fast() cannot copy more than 8 bytes at a time : it doesn't know input length, and only relies on end-of-block properties */ ip += length; op = cpy; } else { cpy = op+length; @@ -1681,7 +1687,12 @@ LZ4_decompress_generic( goto safe_literal_copy; } /* Literals can only be 14, but hope compilers optimize if we copy by a register size */ - memcpy(op, ip, 16); + if (endOnInput) + memcpy(op, ip, 16); + else { /* LZ4_decompress_fast() cannot copy more than 8 bytes at a time : it doesn't know input length, and only relies on end-of-block properties */ + memcpy(op, ip, 8); + if (length > 8) memcpy(op+8, ip+8, 8); + } ip += length; op = cpy; } diff --git a/lib/lz4.h b/lib/lz4.h index 1589be9..962f5e6 100644 --- a/lib/lz4.h +++ b/lib/lz4.h @@ -631,14 +631,15 @@ LZ4_DEPRECATED("use LZ4_decompress_safe_usingDict() instead") LZ4LIB_API int LZ4 LZ4_DEPRECATED("use LZ4_decompress_fast_usingDict() instead") LZ4LIB_API int LZ4_decompress_fast_withPrefix64k (const char* src, char* dst, int originalSize); /*! LZ4_decompress_fast() : **unsafe!** - * These functions are generally slightly faster than LZ4_decompress_safe(), - * though the difference is small (generally ~5%). - * However, the real cost is a risk : LZ4_decompress_safe() is protected vs malformed input, while `LZ4_decompress_fast()` is not, making it a security liability. + * These functions used to be faster than LZ4_decompress_safe(), + * but it has changed, and they are now slower than LZ4_decompress_safe(). + * This is because LZ4_decompress_fast() doesn't know the input size, + * and therefore must progress more cautiously in the input buffer to not read beyond the end of block. + * On top of that `LZ4_decompress_fast()` is not protected vs malformed or malicious inputs, making it a security liability. * As a consequence, LZ4_decompress_fast() is strongly discouraged, and deprecated. - * These functions will generate a deprecation warning in the future. * - * Last LZ4_decompress_fast() specificity is that it can decompress a block without knowing its compressed size. - * Note that even that functionality could be achieved in a more secure manner if need be, + * Only LZ4_decompress_fast() specificity is that it can decompress a block without knowing its compressed size. + * Even that functionality could be achieved in a more secure manner if need be, * though it would require new prototypes, and adaptation of the implementation to this new use case. * * Parameters: @@ -655,11 +656,11 @@ LZ4_DEPRECATED("use LZ4_decompress_fast_usingDict() instead") LZ4LIB_API int LZ4 * As a consequence, use these functions in trusted environments with trusted data **only**. */ -/* LZ4_DEPRECATED("This function is deprecated and unsafe. Consider using LZ4_decompress_safe() instead") */ +LZ4_DEPRECATED("This function is deprecated and unsafe. Consider using LZ4_decompress_safe() instead") LZ4LIB_API int LZ4_decompress_fast (const char* src, char* dst, int originalSize); -/* LZ4_DEPRECATED("This function is deprecated and unsafe. Consider using LZ4_decompress_safe_continue() instead") */ +LZ4_DEPRECATED("This function is deprecated and unsafe. Consider using LZ4_decompress_safe_continue() instead") LZ4LIB_API int LZ4_decompress_fast_continue (LZ4_streamDecode_t* LZ4_streamDecode, const char* src, char* dst, int originalSize); -/* LZ4_DEPRECATED("This function is deprecated and unsafe. Consider using LZ4_decompress_safe_usingDict() instead") */ +LZ4_DEPRECATED("This function is deprecated and unsafe. Consider using LZ4_decompress_safe_usingDict() instead") LZ4LIB_API int LZ4_decompress_fast_usingDict (const char* src, char* dst, int originalSize, const char* dictStart, int dictSize); /*! LZ4_resetStream() : -- cgit v0.12