commit 9d981f8dbf9b0b7b282bcc829b94da5f5d0bc4de
author Lei Zhang <thestig@chromium.org> Tue Jun 25 17:12:53 2024 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Jun 25 17:12:53 2024 +0000
tree de1c0ff46514fa59e0a29e0ca018b1ef39881d5b
parent 09635f35dc56ea24ec2a2724cc8e08573bb06864

Add strict_mode parameter to CJPX_Decoder::Create()

Allow pdf_jpx_fuzzer to exercise the JPEG2000 decoding code, without
enabling the new code paths exposed by calling
opj_decoder_set_strict_mode() by default.

Bug: 42270564, 347071498, 348129258
Change-Id: Iabb7be6e098b322a7c7c916e01fca4a8f55d9633
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/120930
Reviewed-by: Thomas Sepez <tsepez@google.com>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

core/fpdfapi/page/cpdf_dib.cpp

diff --git a/core/fpdfapi/page/cpdf_dib.cpp b/core/fpdfapi/page/cpdf_dib.cpp
index 22733de..421af3f 100644
--- a/core/fpdfapi/page/cpdf_dib.cpp
+++ b/core/fpdfapi/page/cpdf_dib.cpp
@@ -661,7 +661,7 @@
   std::unique_ptr<CJPX_Decoder> decoder =
       CJPX_Decoder::Create(m_pStreamAcc->GetSpan(),
                            ColorSpaceOptionFromColorSpace(m_pColorSpace.Get()),
-                           resolution_levels_to_skip);
+                           resolution_levels_to_skip, /*strict_mode=*/true);
   if (!decoder)
     return nullptr;
 

core/fxcodec/jpx/cjpx_decoder.cpp

diff --git a/core/fxcodec/jpx/cjpx_decoder.cpp b/core/fxcodec/jpx/cjpx_decoder.cpp
index 33157b5..8a53dd9 100644
--- a/core/fxcodec/jpx/cjpx_decoder.cpp
+++ b/core/fxcodec/jpx/cjpx_decoder.cpp
@@ -413,11 +413,13 @@
 std::unique_ptr<CJPX_Decoder> CJPX_Decoder::Create(
     pdfium::span<const uint8_t> src_span,
     CJPX_Decoder::ColorSpaceOption option,
-    uint8_t resolution_levels_to_skip) {
+    uint8_t resolution_levels_to_skip,
+    bool strict_mode) {
   // Private ctor.
   auto decoder = pdfium::WrapUnique(new CJPX_Decoder(option));
-  if (!decoder->Init(src_span, resolution_levels_to_skip))
+  if (!decoder->Init(src_span, resolution_levels_to_skip, strict_mode)) {
     return nullptr;
+  }
   return decoder;
 }
 
@@ -432,7 +434,8 @@
 CJPX_Decoder::~CJPX_Decoder() = default;
 
 bool CJPX_Decoder::Init(pdfium::span<const uint8_t> src_data,
-                        uint8_t resolution_levels_to_skip) {
+                        uint8_t resolution_levels_to_skip,
+                        bool strict_mode) {
   static constexpr uint8_t kJP2Header[] = {0x00, 0x00, 0x00, 0x0c, 0x6a, 0x50,
                                            0x20, 0x20, 0x0d, 0x0a, 0x87, 0x0a};
   if (src_data.size() < sizeof(kJP2Header) ||
@@ -470,6 +473,11 @@
     return false;
   }
 
+  // For https://crbug.com/42270564
+  if (!strict_mode) {
+    CHECK(opj_decoder_set_strict_mode(m_Codec.get(), false));
+  }
+
   opj_image_t* pTempImage = nullptr;
   if (!opj_read_header(m_Stream.get(), m_Codec.get(), &pTempImage)) {
     return false;

core/fxcodec/jpx/cjpx_decoder.h

diff --git a/core/fxcodec/jpx/cjpx_decoder.h b/core/fxcodec/jpx/cjpx_decoder.h
index fb1efd4..a693413 100644
--- a/core/fxcodec/jpx/cjpx_decoder.h
+++ b/core/fxcodec/jpx/cjpx_decoder.h
@@ -46,7 +46,8 @@
   static std::unique_ptr<CJPX_Decoder> Create(
       pdfium::span<const uint8_t> src_span,
       CJPX_Decoder::ColorSpaceOption option,
-      uint8_t resolution_levels_to_skip);
+      uint8_t resolution_levels_to_skip,
+      bool strict_mode);
 
   static void Sycc420ToRgbForTesting(opj_image_t* img);
 
@@ -87,8 +88,11 @@
   // Use Create() to instantiate.
   explicit CJPX_Decoder(ColorSpaceOption option);
 
+  // TODO(crbug.com/42270564): Remove `strict_mode` once all the bugs have been
+  // worked out in OpenJPEG.
   bool Init(pdfium::span<const uint8_t> src_data,
-            uint8_t resolution_levels_to_skip);
+            uint8_t resolution_levels_to_skip,
+            bool strict_mode);
 
   const ColorSpaceOption m_ColorSpaceOption;
   pdfium::raw_span<const uint8_t> m_SrcData;

testing/fuzzers/pdf_jpx_fuzzer.cc

diff --git a/testing/fuzzers/pdf_jpx_fuzzer.cc b/testing/fuzzers/pdf_jpx_fuzzer.cc
index 5306f4d..3fc45ef 100644
--- a/testing/fuzzers/pdf_jpx_fuzzer.cc
+++ b/testing/fuzzers/pdf_jpx_fuzzer.cc
@@ -26,15 +26,21 @@
 }  // namespace
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
-  if (size < 2)
+  if (size < 3) {
     return 0;
+  }
 
   // SAFETY: trusted arguments from fuzzer.
   auto span = UNSAFE_BUFFERS(pdfium::make_span(data, size));
 
-  std::unique_ptr<CJPX_Decoder> decoder = CJPX_Decoder::Create(
-      span.subspan(2u),
-      static_cast<CJPX_Decoder::ColorSpaceOption>(data[0] % 3), data[1]);
+  auto color_space_option =
+      static_cast<CJPX_Decoder::ColorSpaceOption>(data[0] % 3);
+  uint8_t resolution_levels_to_skip = data[1];
+  bool strict_mode = !!data[2];
+
+  std::unique_ptr<CJPX_Decoder> decoder =
+      CJPX_Decoder::Create(span.subspan(3u), color_space_option,
+                           resolution_levels_to_skip, strict_mode);
   if (!decoder)
     return 0;