diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index e1f7538..5a544bd 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -18,3 +18,5 @@ d740c6ccced02147f84a39d5e5f0984d12bac6cb 9ae7bdea966102f9621b22192747a891078e7470 # Normalize white space in ChangeLog d7b909f97d3effc9540c35b0251bdf1c9abf187c +# Remove 'this->' +f5cac93ac63559ddc0aec1b1c61c9e2503c7b8e0 diff --git a/fuzz/CMakeLists.txt b/fuzz/CMakeLists.txt index 9a30b3b..025a18c 100644 --- a/fuzz/CMakeLists.txt +++ b/fuzz/CMakeLists.txt @@ -154,6 +154,7 @@ set(CORPUS_OTHER 389974979.fuzz 391974927.fuzz 394129398.fuzz + 394463491.fuzz ) set(CORPUS_DIR ${CMAKE_CURRENT_BINARY_DIR}/qpdf_corpus) diff --git a/fuzz/qpdf_crypt_fuzzer.cc b/fuzz/qpdf_crypt_fuzzer.cc index 54aba80..f310b50 100644 --- a/fuzz/qpdf_crypt_fuzzer.cc +++ b/fuzz/qpdf_crypt_fuzzer.cc @@ -111,7 +111,7 @@ FuzzHelper::doChecks() Pl_PNGFilter::setMemoryLimit(1'000'000); Pl_RunLength::setMemoryLimit(1'000'000); Pl_TIFFPredictor::setMemoryLimit(1'000'000); - Pl_Flate::setMemoryLimit(200'000); + Pl_Flate::memory_limit(200'000); // Do not decompress corrupt data. This may cause extended runtime within jpeglib without // exercising additional code paths in qpdf, and potentially causing counterproductive timeouts. diff --git a/fuzz/qpdf_crypt_insecure_fuzzer.cc b/fuzz/qpdf_crypt_insecure_fuzzer.cc index 61c55c5..76d1337 100644 --- a/fuzz/qpdf_crypt_insecure_fuzzer.cc +++ b/fuzz/qpdf_crypt_insecure_fuzzer.cc @@ -111,7 +111,7 @@ FuzzHelper::doChecks() Pl_PNGFilter::setMemoryLimit(1'000'000); Pl_RunLength::setMemoryLimit(1'000'000); Pl_TIFFPredictor::setMemoryLimit(1'000'000); - Pl_Flate::setMemoryLimit(200'000); + Pl_Flate::memory_limit(200'000); // Do not decompress corrupt data. This may cause extended runtime within jpeglib without // exercising additional code paths in qpdf, and potentially causing counterproductive timeouts. diff --git a/fuzz/qpdf_extra/394463491.fuzz b/fuzz/qpdf_extra/394463491.fuzz new file mode 100644 index 0000000..7a74fe3 --- /dev/null +++ b/fuzz/qpdf_extra/394463491.fuzz diff --git a/fuzz/qpdf_fuzzer.cc b/fuzz/qpdf_fuzzer.cc index a066b13..65aac38 100644 --- a/fuzz/qpdf_fuzzer.cc +++ b/fuzz/qpdf_fuzzer.cc @@ -109,7 +109,7 @@ FuzzHelper::doChecks() Pl_PNGFilter::setMemoryLimit(1'000'000); Pl_RunLength::setMemoryLimit(1'000'000); Pl_TIFFPredictor::setMemoryLimit(1'000'000); - Pl_Flate::setMemoryLimit(200'000); + Pl_Flate::memory_limit(200'000); // Do not decompress corrupt data. This may cause extended runtime within jpeglib without // exercising additional code paths in qpdf, and potentially causing counterproductive timeouts. diff --git a/fuzz/qpdf_lin_fuzzer.cc b/fuzz/qpdf_lin_fuzzer.cc index f116eec..3be6a7d 100644 --- a/fuzz/qpdf_lin_fuzzer.cc +++ b/fuzz/qpdf_lin_fuzzer.cc @@ -110,7 +110,7 @@ FuzzHelper::doChecks() Pl_PNGFilter::setMemoryLimit(1'000'000); Pl_RunLength::setMemoryLimit(1'000'000); Pl_TIFFPredictor::setMemoryLimit(1'000'000); - Pl_Flate::setMemoryLimit(200'000); + Pl_Flate::memory_limit(200'000); // Do not decompress corrupt data. This may cause extended runtime within jpeglib without // exercising additional code paths in qpdf, and potentially causing counterproductive timeouts. diff --git a/fuzz/qpdf_outlines_fuzzer.cc b/fuzz/qpdf_outlines_fuzzer.cc index f1887f7..7a2f791 100644 --- a/fuzz/qpdf_outlines_fuzzer.cc +++ b/fuzz/qpdf_outlines_fuzzer.cc @@ -87,7 +87,7 @@ FuzzHelper::doChecks() Pl_PNGFilter::setMemoryLimit(1'000'000); Pl_RunLength::setMemoryLimit(1'000'000); Pl_TIFFPredictor::setMemoryLimit(1'000'000); - Pl_Flate::setMemoryLimit(200'000); + Pl_Flate::memory_limit(200'000); // Do not decompress corrupt data. This may cause extended runtime within jpeglib without // exercising additional code paths in qpdf, and potentially causing counterproductive timeouts. diff --git a/fuzz/qpdf_pages_fuzzer.cc b/fuzz/qpdf_pages_fuzzer.cc index 9fb3baf..f1a0db1 100644 --- a/fuzz/qpdf_pages_fuzzer.cc +++ b/fuzz/qpdf_pages_fuzzer.cc @@ -108,7 +108,7 @@ FuzzHelper::doChecks() Pl_PNGFilter::setMemoryLimit(1'000'000); Pl_RunLength::setMemoryLimit(1'000'000); Pl_TIFFPredictor::setMemoryLimit(1'000'000); - Pl_Flate::setMemoryLimit(200'000); + Pl_Flate::memory_limit(200'000); // Do not decompress corrupt data. This may cause extended runtime within jpeglib without // exercising additional code paths in qpdf, and potentially causing counterproductive timeouts. diff --git a/fuzz/qtest/fuzz.test b/fuzz/qtest/fuzz.test index 9a784d1..eb943c5 100644 --- a/fuzz/qtest/fuzz.test +++ b/fuzz/qtest/fuzz.test @@ -11,7 +11,7 @@ my $td = new TestDriver('fuzz'); my $qpdf_corpus = $ENV{'QPDF_FUZZ_CORPUS'} || die "must set QPDF_FUZZ_CORPUS"; -my $n_qpdf_files = 91; # increment when adding new files +my $n_qpdf_files = 92; # increment when adding new files my @fuzzers = ( ['ascii85' => 1], diff --git a/include/qpdf/Pl_Flate.hh b/include/qpdf/Pl_Flate.hh index 91c3c54..c7bbeba 100644 --- a/include/qpdf/Pl_Flate.hh +++ b/include/qpdf/Pl_Flate.hh @@ -48,7 +48,9 @@ class QPDF_DLL_CLASS Pl_Flate: public Pipeline // Limit the memory used. // NB This is a static option affecting all Pl_Flate instances. QPDF_DLL - static void setMemoryLimit(unsigned long long limit); + static unsigned long long memory_limit(); + QPDF_DLL + static void memory_limit(unsigned long long limit); QPDF_DLL void write(unsigned char const* data, size_t len) override; diff --git a/libqpdf/Pl_Flate.cc b/libqpdf/Pl_Flate.cc index 6e690cb..97a337b 100644 --- a/libqpdf/Pl_Flate.cc +++ b/libqpdf/Pl_Flate.cc @@ -14,7 +14,7 @@ namespace { - unsigned long long memory_limit{0}; + unsigned long long memory_limit_{0}; } // namespace int Pl_Flate::compression_level = Z_DEFAULT_COMPRESSION; @@ -80,10 +80,16 @@ Pl_Flate::~Pl_Flate() // NOLINT (modernize-use-equals-default) // Must be explicit and not inline -- see QPDF_DLL_CLASS in README-maintainer } +unsigned long long +Pl_Flate::memory_limit() +{ + return memory_limit_; +} + void -Pl_Flate::setMemoryLimit(unsigned long long limit) +Pl_Flate::memory_limit(unsigned long long limit) { - memory_limit = limit; + memory_limit_ = limit; } void @@ -197,9 +203,9 @@ Pl_Flate::handleData(unsigned char const* data, size_t len, int flush) } uLong ready = QIntC::to_ulong(m->out_bufsize - zstream.avail_out); if (ready > 0) { - if (memory_limit && m->action != a_deflate) { + if (memory_limit_ && m->action != a_deflate) { m->written += ready; - if (m->written > memory_limit) { + if (m->written > memory_limit_) { throw std::runtime_error("PL_Flate memory limit exceeded"); } } @@ -220,7 +226,7 @@ Pl_Flate::handleData(unsigned char const* data, size_t len, int flush) void Pl_Flate::finish() { - if (m->written > memory_limit) { + if (m->written > memory_limit_) { throw std::runtime_error("PL_Flate memory limit exceeded"); } try { diff --git a/libqpdf/QPDFJob_argv.cc b/libqpdf/QPDFJob_argv.cc index 9aeabc0..180a7a6 100644 --- a/libqpdf/QPDFJob_argv.cc +++ b/libqpdf/QPDFJob_argv.cc @@ -117,8 +117,10 @@ ArgParser::argZopfli() logger->info("Set the environment variable QPDF_ZOPFLI to activate.\n"); logger->info("* QPDF_ZOPFLI=disabled or QPDF_ZOPFLI not set: don't use zopfli.\n"); logger->info("* QPDF_ZOPFLI=force: use zopfli, and fail if not available.\n"); - logger->info("* QPDF_ZOPFLI=silent: use zopfli if available and silently fall back if not.\n"); - logger->info("* QPDF_ZOPFLI= any other value: use zopfli if available, and warn if not.\n"); + logger->info( + "* QPDF_ZOPFLI=silent: use zopfli if available and silently fall back if not.\n"); + logger->info( + "* QPDF_ZOPFLI= any other value: use zopfli if available, and warn if not.\n"); } } else { logger->error("zopfli support is not enabled\n"); diff --git a/libqpdf/SF_FlateLzwDecode.cc b/libqpdf/SF_FlateLzwDecode.cc index a291145..433d585 100644 --- a/libqpdf/SF_FlateLzwDecode.cc +++ b/libqpdf/SF_FlateLzwDecode.cc @@ -7,17 +7,6 @@ #include #include -SF_FlateLzwDecode::SF_FlateLzwDecode(bool lzw) : - lzw(lzw), - // Initialize values to their defaults as per the PDF spec - predictor(1), - columns(1), - colors(1), - bits_per_component(8), - early_code_change(true) -{ -} - bool SF_FlateLzwDecode::setDecodeParms(QPDFObjectHandle decode_parms) { @@ -25,78 +14,83 @@ SF_FlateLzwDecode::setDecodeParms(QPDFObjectHandle decode_parms) return true; } - bool filterable = true; + auto memory_limit = Pl_Flate::memory_limit(); + std::set keys = decode_parms.getKeys(); for (auto const& key: keys) { QPDFObjectHandle value = decode_parms.getKey(key); if (key == "/Predictor") { if (value.isInteger()) { - this->predictor = value.getIntValueAsInt(); - if (!((this->predictor == 1) || (this->predictor == 2) || - ((this->predictor >= 10) && (this->predictor <= 15)))) { - filterable = false; + predictor = value.getIntValueAsInt(); + if (!(predictor == 1 || predictor == 2 || (predictor >= 10 && predictor <= 15))) { + return false; } } else { - filterable = false; + return false; } - } else if ((key == "/Columns") || (key == "/Colors") || (key == "/BitsPerComponent")) { + } else if (key == "/Columns" || key == "/Colors" || key == "/BitsPerComponent") { if (value.isInteger()) { int val = value.getIntValueAsInt(); + if (memory_limit && static_cast(val) > memory_limit) { + QPDFLogger::defaultLogger()->warn( + "SF_FlateLzwDecode parameter exceeds PL_Flate memory limit\n"); + return false; + } if (key == "/Columns") { - this->columns = val; + columns = val; } else if (key == "/Colors") { - this->colors = val; + colors = val; } else if (key == "/BitsPerComponent") { - this->bits_per_component = val; + bits_per_component = val; } } else { - filterable = false; + return false; } } else if (lzw && (key == "/EarlyChange")) { if (value.isInteger()) { int earlychange = value.getIntValueAsInt(); - this->early_code_change = (earlychange == 1); - if (!((earlychange == 0) || (earlychange == 1))) { - filterable = false; + early_code_change = (earlychange == 1); + if (!(earlychange == 0 || earlychange == 1)) { + return false; } } else { - filterable = false; + return false; } } } - if ((this->predictor > 1) && (this->columns == 0)) { - filterable = false; + if (predictor > 1 && columns == 0) { + return false; } - return filterable; + return true; } Pipeline* SF_FlateLzwDecode::getDecodePipeline(Pipeline* next) { std::shared_ptr pipeline; - if ((this->predictor >= 10) && (this->predictor <= 15)) { + if (predictor >= 10 && predictor <= 15) { QTC::TC("qpdf", "SF_FlateLzwDecode PNG filter"); pipeline = std::make_shared( "png decode", next, Pl_PNGFilter::a_decode, - QIntC::to_uint(this->columns), - QIntC::to_uint(this->colors), - QIntC::to_uint(this->bits_per_component)); - this->pipelines.push_back(pipeline); + QIntC::to_uint(columns), + QIntC::to_uint(colors), + QIntC::to_uint(bits_per_component)); + pipelines.push_back(pipeline); next = pipeline.get(); - } else if (this->predictor == 2) { + } else if (predictor == 2) { QTC::TC("qpdf", "SF_FlateLzwDecode TIFF predictor"); pipeline = std::make_shared( "tiff decode", next, Pl_TIFFPredictor::a_decode, - QIntC::to_uint(this->columns), - QIntC::to_uint(this->colors), - QIntC::to_uint(this->bits_per_component)); - this->pipelines.push_back(pipeline); + QIntC::to_uint(columns), + QIntC::to_uint(colors), + QIntC::to_uint(bits_per_component)); + pipelines.push_back(pipeline); next = pipeline.get(); } @@ -105,7 +99,7 @@ SF_FlateLzwDecode::getDecodePipeline(Pipeline* next) } else { pipeline = std::make_shared("stream inflate", next, Pl_Flate::a_inflate); } - this->pipelines.push_back(pipeline); + pipelines.push_back(pipeline); return pipeline.get(); } diff --git a/libqpdf/qpdf/SF_FlateLzwDecode.hh b/libqpdf/qpdf/SF_FlateLzwDecode.hh index 203d893..4b70b50 100644 --- a/libqpdf/qpdf/SF_FlateLzwDecode.hh +++ b/libqpdf/qpdf/SF_FlateLzwDecode.hh @@ -5,25 +5,29 @@ #ifndef SF_FLATELZWDECODE_HH # define SF_FLATELZWDECODE_HH -class SF_FlateLzwDecode: public QPDFStreamFilter +class SF_FlateLzwDecode final: public QPDFStreamFilter { public: - SF_FlateLzwDecode(bool lzw); - ~SF_FlateLzwDecode() override = default; + SF_FlateLzwDecode(bool lzw) : + lzw(lzw) + { + } + ~SF_FlateLzwDecode() final = default; - bool setDecodeParms(QPDFObjectHandle decode_parms) override; - Pipeline* getDecodePipeline(Pipeline* next) override; + bool setDecodeParms(QPDFObjectHandle decode_parms) final; + Pipeline* getDecodePipeline(Pipeline* next) final; static std::shared_ptr flate_factory(); static std::shared_ptr lzw_factory(); private: - bool lzw; - int predictor; - int columns; - int colors; - int bits_per_component; - bool early_code_change; + bool lzw{}; + // Defaults as per the PDF spec + int predictor{1}; + int columns{1}; + int colors{1}; + int bits_per_component{8}; + bool early_code_change{true}; std::vector> pipelines; };