From 25642dfe87eb0b386c1df784436f0588264d73f0 Mon Sep 17 00:00:00 2001
From: Tim Schumacher <timschumi@gmx.de>
Date: Tue, 24 Oct 2023 10:57:50 +0200
Subject: [PATCH] LibCompress: Implement correct validation of last filters

---
 Tests/LibCompress/TestXz.cpp          |  3 ---
 Userland/Libraries/LibCompress/Xz.cpp | 13 +++++++++++--
 Userland/Libraries/LibCompress/Xz.h   |  2 +-
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/Tests/LibCompress/TestXz.cpp b/Tests/LibCompress/TestXz.cpp
index 7a70be35073..dcaad5e9a1f 100644
--- a/Tests/LibCompress/TestXz.cpp
+++ b/Tests/LibCompress/TestXz.cpp
@@ -1864,9 +1864,6 @@ TEST_CASE(xz_utils_unsupported_filter_flags_2)
     auto stream = MUST(try_make<FixedMemoryStream>(compressed));
     auto decompressor = MUST(Compress::XzDecompressor::create(move(stream)));
 
-    // TODO: We don't yet check whether the filter chain satisfies the "can't be the last filter"
-    //       requirement. We just happen to get the result right because we try to uncompress the
-    //       test case and fail.
     auto buffer_or_error = decompressor->read_until_eof(PAGE_SIZE);
     EXPECT(buffer_or_error.is_error());
 }
diff --git a/Userland/Libraries/LibCompress/Xz.cpp b/Userland/Libraries/LibCompress/Xz.cpp
index f928d1ad81b..f07c022ef2f 100644
--- a/Userland/Libraries/LibCompress/Xz.cpp
+++ b/Userland/Libraries/LibCompress/Xz.cpp
@@ -121,7 +121,7 @@ u32 XzStreamFooter::backward_size() const
     return (encoded_backward_size + 1) * 4;
 }
 
-u8 XzBlockFlags::number_of_filters() const
+size_t XzBlockFlags::number_of_filters() const
 {
     // 3.1.2. Block Flags:
     // "Bit(s)  Mask  Description
@@ -380,6 +380,7 @@ ErrorOr<void> XzDecompressor::load_next_block(u8 encoded_block_header_size)
     struct FilterEntry {
         u64 id;
         ByteBuffer properties;
+        bool last;
     };
     Vector<FilterEntry, 4> filters;
 
@@ -387,6 +388,8 @@ ErrorOr<void> XzDecompressor::load_next_block(u8 encoded_block_header_size)
     // "The number of Filter Flags fields is stored in the Block Flags
     //  field (see Section 3.1.2)."
     for (size_t i = 0; i < flags.number_of_filters(); i++) {
+        auto last = (i == flags.number_of_filters() - 1);
+
         // "The format of each Filter Flags field is as follows:
         //  Both Filter ID and Size of Properties are stored using the
         //  encoding described in Section 1.2."
@@ -397,12 +400,15 @@ ErrorOr<void> XzDecompressor::load_next_block(u8 encoded_block_header_size)
         auto filter_properties = TRY(ByteBuffer::create_uninitialized(size_of_properties));
         TRY(header_stream.read_until_filled(filter_properties));
 
-        filters.empend(filter_id, move(filter_properties));
+        filters.empend(filter_id, move(filter_properties), last);
     }
 
     for (auto& filter : filters.in_reverse()) {
         // 5.3.1. LZMA2
         if (filter.id == 0x21) {
+            if (!filter.last)
+                return Error::from_string_literal("XZ LZMA2 filter can only be the last filter");
+
             if (filter.properties.size() < sizeof(XzFilterLzma2Properties))
                 return Error::from_string_literal("XZ LZMA2 filter has a smaller-than-needed properties size");
 
@@ -415,6 +421,9 @@ ErrorOr<void> XzDecompressor::load_next_block(u8 encoded_block_header_size)
 
         // 5.3.3. Delta
         if (filter.id == 0x03) {
+            if (filter.last)
+                return Error::from_string_literal("XZ Delta filter can only be a non-last filter");
+
             if (filter.properties.size() < sizeof(XzFilterDeltaProperties))
                 return Error::from_string_literal("XZ Delta filter has a smaller-than-needed properties size");
 
diff --git a/Userland/Libraries/LibCompress/Xz.h b/Userland/Libraries/LibCompress/Xz.h
index 8a054eb0b48..aa9e69fd987 100644
--- a/Userland/Libraries/LibCompress/Xz.h
+++ b/Userland/Libraries/LibCompress/Xz.h
@@ -85,7 +85,7 @@ struct [[gnu::packed]] XzBlockFlags {
     bool compressed_size_present : 1;
     bool uncompressed_size_present : 1;
 
-    u8 number_of_filters() const;
+    size_t number_of_filters() const;
 };
 static_assert(sizeof(XzBlockFlags) == 1);