From 5f5ca4db28f83cf491251c892f99e796f14263c4 Mon Sep 17 00:00:00 2001
From: Pascal Seitz <pascal.seitz@gmail.com>
Date: Sun, 30 Apr 2023 13:30:26 +0800
Subject: [PATCH] fix: checked-decode

fixes checked decode checks
revert https://github.com/PSeitz/lz4_flex/pull/69 as this leads to out of bounds writes
---
 src/block/decompress.rs      |  2 +-
 src/block/decompress_safe.rs | 11 +++++++++--
 tests/tests.rs               | 18 +++++++++++++++---
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/src/block/decompress.rs b/src/block/decompress.rs
index 07d28f5f..7af26f74 100644
--- a/src/block/decompress.rs
+++ b/src/block/decompress.rs
@@ -63,7 +63,7 @@ unsafe fn duplicate_overlapping(
         // Note that we copy 4 bytes, instead of one.
         // Without that the compiler will unroll/auto-vectorize the copy with a lot of branches.
         // This is not what we want, as large overlapping copies are not that common.
-        core::ptr::copy(start, *output_ptr, 4);
+        core::ptr::copy(start, *output_ptr, 1);
         start = start.add(1);
         *output_ptr = output_ptr.add(1);
     }
diff --git a/src/block/decompress_safe.rs b/src/block/decompress_safe.rs
index 4dd22ee1..17cc316a 100644
--- a/src/block/decompress_safe.rs
+++ b/src/block/decompress_safe.rs
@@ -149,7 +149,10 @@ pub(crate) fn decompress_internal<const USE_DICT: bool, S: Sink>(
             // In this branch we know that match_length is at most 18 (14 + MINMATCH).
             // But the blocks can overlap, so make sure they are at least 18 bytes apart
             // to enable an optimized copy of 18 bytes.
-            let start = output.pos() - offset;
+            let (start, did_overflow) = output.pos().overflowing_sub(offset);
+            if did_overflow {
+                return Err(DecompressError::OffsetOutOfBounds);
+            }
             if offset >= match_length {
                 output.extend_from_within(start, 18, match_length);
             } else {
@@ -261,7 +264,11 @@ fn duplicate_slice(
     if match_length > offset {
         duplicate_overlapping_slice(output, offset, match_length)?;
     } else {
-        let start = output.pos() - offset;
+        let (start, did_overflow) = output.pos().overflowing_sub(offset);
+        if did_overflow {
+            return Err(DecompressError::OffsetOutOfBounds);
+        }
+
         match match_length {
             0..=32 if output.pos() + 32 <= output.capacity() => {
                 output.extend_from_within(start, 32, match_length)
diff --git a/tests/tests.rs b/tests/tests.rs
index ce6f4d9b..bc836755 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -322,25 +322,37 @@ fn print_compression_ration(input: &'static [u8], name: &str) {
 mod checked_decode {
     use super::*;
 
-    #[cfg_attr(not(feature = "checked_decode"), ignore)]
+    #[cfg_attr(not(feature = "checked-decode"), ignore)]
     #[test]
     fn error_case_1() {
         let _err = decompress_size_prepended(&[122, 1, 0, 1, 0, 10, 1, 0]);
     }
-    #[cfg_attr(not(feature = "checked_decode"), ignore)]
+    #[cfg_attr(not(feature = "checked-decode"), ignore)]
     #[test]
     fn error_case_2() {
         let _err = decompress_size_prepended(&[
             44, 251, 49, 0, 0, 0, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 16, 0, 0, 0, 0, 0, 0, 0, 0,
         ]);
     }
-    #[cfg_attr(not(feature = "checked_decode"), ignore)]
+    #[cfg_attr(not(feature = "checked-decode"), ignore)]
     #[test]
     fn error_case_3() {
         let _err = decompress_size_prepended(&[
             7, 0, 0, 0, 0, 0, 0, 11, 0, 0, 7, 16, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 1, 0, 0,
         ]);
     }
+
+    #[cfg_attr(not(feature = "checked-decode"), ignore)]
+    #[test]
+    fn error_case_4() {
+        let _err = decompress_size_prepended(&[0, 61, 0, 0, 0, 7, 0]);
+    }
+
+    #[cfg_attr(not(feature = "checked-decode"), ignore)]
+    #[test]
+    fn error_case_5() {
+        let _err = decompress_size_prepended(&[8, 0, 0, 0, 4, 0, 0, 0]);
+    }
 }
 
 #[test]