Prevent endless loop during decoding for invalid JPEG-LS input data (#334)

Fuzzing testing of the C# implementation found a scenario that would cause an endless loop during decoding. This defect was also present in the C++ implementation.
This condition was tested with an ASSERT. Using an ASSERT is fine when encoding, but an explicit check is needed when decoding as the input data is in that case untrusted.

CHANGELOG.md

@@ -12,6 +12,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
 - Support to retrieve the height from a DNL marker segment.
 - Support to encode and decode mixed interleaved mode scans.
 
+### Fixed
+
+- Endless loop when decoding invalid JPEG-LS input data.
+
 ### Changed
 
 - BREAKING: Updated the minimal required C++ language version to C++17.

src/run_mode_context.hpp

@@ -32,6 +32,21 @@ public:
         return run_interruption_type_;
     }
 
+    [[nodiscard]]
+    FORCE_INLINE int32_t compute_golomb_coding_parameter_checked() const
+    {
+        const int32_t temp{a_ + (n_ >> 1) * run_interruption_type_};
+        int32_t n_test{n_};
+        int32_t k{};
+        for (; n_test < temp; ++k)
+        {
+            n_test <<= 1;
+            if (k > 32)
+                impl::throw_jpegls_error(jpegls_errc::invalid_data);
+        }
+        return k;
+    }
+
     [[nodiscard]]
     FORCE_INLINE int32_t compute_golomb_coding_parameter() const noexcept
     {

src/scan_decoder_impl.hpp

@@ -279,6 +279,7 @@ private:
     {
         const int32_t sign{bit_wise_sign(qs)};
         regular_mode_context& context{regular_mode_contexts_[apply_sign(qs, sign)]};
+        const int32_t corrected_prediction{traits_.correct_prediction(predicted + apply_sign(context.c(), sign))};
         const int32_t k{context.compute_golomb_coding_parameter()};
 
         int32_t error_value;
@@ -301,16 +302,15 @@ private:
             error_value = error_value ^ context.get_error_correction(traits_.near_lossless);
         }
 
-        const int32_t predicted_value{traits_.correct_prediction(predicted + apply_sign(context.c(), sign))};
         context.update_variables_and_bias(error_value, traits_.near_lossless, reset_threshold_);
         error_value = apply_sign(error_value, sign);
-        return traits_.compute_reconstructed_sample(predicted_value, error_value);
+        return traits_.compute_reconstructed_sample(corrected_prediction, error_value);
     }
 
     [[nodiscard]]
     int32_t decode_run_interruption_error(run_mode_context& context)
     {
-        const int32_t k{context.compute_golomb_coding_parameter()};
+        const int32_t k{context.compute_golomb_coding_parameter_checked()};
         const int32_t e_mapped_error_value{
             decode_mapped_error_value(k, traits_.limit - J[run_index_] - 1, traits_.quantized_bits_per_sample)};
         const int32_t error_value{context.compute_error_value(e_mapped_error_value + context.run_interruption_type(), k)};

unittest/CharLSUnitTest.vcxproj

@@ -221,6 +221,15 @@
     </CopyFileToFolders>
     <CopyFileToFolders Include="8bit-monochrome-2x2.jls">
       <FileType>Document</FileType>
+      <DestinationFolders>$(OutDir)DataFiles</DestinationFolders>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="fuzzy-input-bad-run-mode-golomb-code.jls">
+      <FileType>Document</FileType>
+      <DestinationFolders>$(OutDir)DataFiles</DestinationFolders>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="fuzzy-input-no-valid-bits-at-the-end.jls">
+      <FileType>Document</FileType>
+      <DestinationFolders>$(OutDir)DataFiles</DestinationFolders>
     </CopyFileToFolders>
   </ItemGroup>
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />

unittest/CharLSUnitTest.vcxproj.filters

@@ -193,4 +193,12 @@
       <Filter>Test Images</Filter>
     </CopyFileToFolders>
   </ItemGroup>
+  <ItemGroup>
+    <None Include="fuzzy-input-no-valid-bits-at-the-end.jls">
+      <Filter>Test Images</Filter>
+    </None>
+    <None Include="fuzzy-input-bad-run-mode-golomb-code.jls">
+      <Filter>Test Images</Filter>
+    </None>
+  </ItemGroup>
 </Project>

unittest/jpegls_decoder_test.cpp

@@ -237,7 +237,7 @@ public:
 
     TEST_METHOD(get_destination_size_for_small_image_with_custom_stride) // NOLINT
     {
-        const auto source{read_file("8bit-monochrome-2x2.jls")};
+        const auto source{read_file("DataFiles/8bit-monochrome-2x2.jls")};
         jpegls_decoder decoder{source, true};
 
         constexpr uint32_t stride{4};
@@ -266,6 +266,41 @@ public:
         }
     }
 
+    TEST_METHOD(decode_fuzzy_input_no_valid_bits_at_the_end_throws) // NOLINT
+    {
+        // Remark: exception is thrown from different location when lossless_traits becomes more generic.
+        const auto source{read_file("DataFiles/fuzzy-input-no-valid-bits-at-the-end.jls")};
+        jpegls_decoder decoder{source, true};
+
+        vector<byte> destination(decoder.get_destination_size());
+
+        assert_expect_exception(jpegls_errc::invalid_data, [&decoder, &destination] { decoder.decode(destination); });
+    }
+
+    TEST_METHOD(decode_fuzzy_input_bad_run_mode_golomb_code_throws) // NOLINT
+    {
+        const auto source{read_file("DataFiles/fuzzy-input-bad-run-mode-golomb-code.jls")};
+        jpegls_decoder decoder{source, true};
+
+        vector<byte> destination(decoder.get_destination_size());
+
+        assert_expect_exception(jpegls_errc::invalid_data, [&decoder, &destination] { decoder.decode(destination); });
+    }
+
+    TEST_METHOD(get_destination_size_returns_zero_for_abbreviated_table_specification) // NOLINT
+    {
+        const vector<byte> table_data(4);
+        jpeg_test_stream_writer writer;
+        writer.write_start_of_image();
+        writer.write_jpegls_preset_parameters_segment(1, 1, table_data, false);
+        writer.write_marker(jpeg_marker_code::end_of_image);
+
+        const jpegls_decoder decoder{writer.buffer, true};
+        const auto size = decoder.get_destination_size();
+
+        Assert::AreEqual(size_t{0}, size);
+    }
+
     TEST_METHOD(decode_with_default_pc_parameters_before_each_sos) // NOLINT
     {
         auto source{read_file("DataFiles/t8c0e0.jls")};
@@ -1040,7 +1075,7 @@ public:
     {
         constexpr frame_info frame_info{100, 100, 8, 1};
         const vector<byte> source(static_cast<size_t>(frame_info.width) * frame_info.height);
-        
+
         jpegls_encoder encoder;
         encoder.frame_info(frame_info);
         encoder.set_mapping_table_id(0, 1);

unittest/run_mode_context_test.cpp

@@ -20,6 +20,7 @@ public:
         context.update_variables(3, 27, 0);
 
         Assert::AreEqual(3, context.compute_golomb_coding_parameter());
+        Assert::AreEqual(3, context.compute_golomb_coding_parameter_checked());
     }
 };