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.

diff --git a/lib/lz4.c b/lib/lz4.c
index bd8fa11..2f8aa04 100644 (file)
--- 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 (file)
--- 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() :