From b46d4b9872654a953c7aedf325d36da2d2acc589 Mon Sep 17 00:00:00 2001 From: m-holger Date: Thu, 7 Aug 2025 19:09:16 +0100 Subject: [PATCH] Disallow `--deterministic-id` with encrypted output and improve error handling for deterministic ID generation (fixes #1235). --- include/qpdf/QPDFWriter.hh | 2 +- libqpdf/QPDFJob_config.cc | 9 +++++++++ libqpdf/QPDFWriter.cc | 20 +++++++++++--------- manual/release-notes.rst | 7 +++++++ qpdf/qpdf.testcov | 1 - qpdf/qtest/arg-parsing.test | 22 +++++++++++++++++++++- qpdf/qtest/deterministic-id.test | 9 ++++----- 7 files changed, 53 insertions(+), 17 deletions(-) diff --git a/include/qpdf/QPDFWriter.hh b/include/qpdf/QPDFWriter.hh index 1dc89b8..be9c32e 100644 --- a/include/qpdf/QPDFWriter.hh +++ b/include/qpdf/QPDFWriter.hh @@ -495,7 +495,7 @@ class QPDFWriter void preserveObjectStreams(); void generateObjectStreams(); std::string getOriginalID1(); - void generateID(); + void generateID(bool encrypted); void interpretR3EncryptionParameters( bool allow_accessibility, bool allow_extract, diff --git a/libqpdf/QPDFJob_config.cc b/libqpdf/QPDFJob_config.cc index f94d52d..1b548f4 100644 --- a/libqpdf/QPDFJob_config.cc +++ b/libqpdf/QPDFJob_config.cc @@ -149,6 +149,9 @@ QPDFJob::Config::jpegQuality(std::string const& parameter) QPDFJob::Config* QPDFJob::Config::copyEncryption(std::string const& parameter) { + if (o.m->deterministic_id) { + usage("the deterministic-id option is incompatible with encrypted output files"); + } o.m->encryption_file = parameter; o.m->copy_encryption = true; o.m->encrypt = false; @@ -168,6 +171,9 @@ QPDFJob::Config::decrypt() QPDFJob::Config* QPDFJob::Config::deterministicId() { + if (o.m->encrypt || o.m->copy_encryption) { + usage("the deterministic-id option is incompatible with encrypted output files"); + } o.m->deterministic_id = true; return this; } @@ -1116,6 +1122,9 @@ std::shared_ptr QPDFJob::Config::encrypt( int keylen, std::string const& user_password, std::string const& owner_password) { + if (o.m->deterministic_id) { + usage("the deterministic-id option is incompatible with encrypted output files"); + } o.m->keylen = keylen; if (keylen == 256) { o.m->use_aes = true; diff --git a/libqpdf/QPDFWriter.cc b/libqpdf/QPDFWriter.cc index 025c84c..44a3aeb 100644 --- a/libqpdf/QPDFWriter.cc +++ b/libqpdf/QPDFWriter.cc @@ -859,7 +859,7 @@ QPDFWriter::interpretR3EncryptionParameters( void QPDFWriter::setEncryptionParameters(char const* user_password, char const* owner_password) { - generateID(); + generateID(true); m->encryption->setId1(m->id1); m->encryption_key = m->encryption->compute_parameters(user_password, owner_password); setEncryptionMinimumVersion(); @@ -871,7 +871,7 @@ QPDFWriter::copyEncryptionParameters(QPDF& qpdf) m->preserve_encryption = false; QPDFObjectHandle trailer = qpdf.getTrailer(); if (trailer.hasKey("/Encrypt")) { - generateID(); + generateID(true); m->id1 = trailer.getKey("/ID").getArrayItem(0).getStringValue(); QPDFObjectHandle encrypt = trailer.getKey("/Encrypt"); int V = encrypt.getKey("/V").getIntValueAsInt(); @@ -1301,7 +1301,7 @@ QPDFWriter::writeTrailer( if (linearization_pass == 0 && m->deterministic_id) { computeDeterministicIDData(); } - generateID(); + generateID(m->encryption.get()); write_string(m->id1, true).write_string(m->id2, true); } write("]"); @@ -1865,7 +1865,7 @@ QPDFWriter::getOriginalID1() } void -QPDFWriter::generateID() +QPDFWriter::generateID(bool encrypted) { // Generate the ID lazily so that we can handle the user's preference to use static or // deterministic ID generation. @@ -1911,12 +1911,14 @@ QPDFWriter::generateID() std::string seed; if (m->deterministic_id) { - if (m->deterministic_id_data.empty()) { - QTC::TC("qpdf", "QPDFWriter deterministic with no data"); + if (encrypted) { throw std::runtime_error( - "INTERNAL ERROR: QPDFWriter::generateID has no data for " - "deterministic ID. This may happen if deterministic ID " - "and file encryption are requested together."); + "QPDFWriter: unable to generated a deterministic ID because the file to be " + "written is encrypted (even though the file may not require a password)"); + } + if (m->deterministic_id_data.empty()) { + throw std::logic_error( + "INTERNAL ERROR: QPDFWriter::generateID has no data for deterministic ID"); } seed += m->deterministic_id_data; } else { diff --git a/manual/release-notes.rst b/manual/release-notes.rst index f61ae3c..b687099 100644 --- a/manual/release-notes.rst +++ b/manual/release-notes.rst @@ -23,6 +23,13 @@ more detail. not work on some older Linux distributions. If you need support for an older distribution, please use version 12.2.0 or below. + - CLI Enhancements + + - Disallow option :qpdf:ref:`--deterministic-id` to be used together + with the incompatible options :qpdf:ref:`--encrypt` or + :qpdf:ref:`--copy-encryption`. + + - Other enhancements - ``QPDFWriter`` will no longer add filters when writing empty streams. diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index c3fd781..c9bc18d 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -254,7 +254,6 @@ QPDFJob npages 0 QPDF already reserved object 0 QPDFWriter standard deterministic ID 1 QPDFWriter linearized deterministic ID 1 -QPDFWriter deterministic with no data 0 qpdf-c called qpdf_set_deterministic_ID 0 QPDFParser invalid objgen 0 QPDF object id 0 0 diff --git a/qpdf/qtest/arg-parsing.test b/qpdf/qtest/arg-parsing.test index 395e469..128826c 100644 --- a/qpdf/qtest/arg-parsing.test +++ b/qpdf/qtest/arg-parsing.test @@ -15,7 +15,7 @@ cleanup(); my $td = new TestDriver('arg-parsing'); -my $n_tests = 26; +my $n_tests = 30; $td->runtest("required argument", {$td->COMMAND => "qpdf --password minimal.pdf"}, @@ -138,6 +138,26 @@ $td->runtest("missing key length", {$td->REGEXP => ".*encryption key length is required", $td->EXIT_STATUS => 2}, $td->NORMALIZE_NEWLINES); +$td->runtest("encrypt and deterministic-id 1", + {$td->COMMAND => "qpdf --deterministic-id --encrypt --"}, + {$td->REGEXP => ".*the deterministic-id option is incompatible with encrypted output files", + $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); +$td->runtest("encrypt and deterministic-id 2", + {$td->COMMAND => "qpdf --deterministic-id --encrypt --bits=256 -- --deterministic-id"}, + {$td->REGEXP => ".*the deterministic-id option is incompatible with encrypted output files", + $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); +$td->runtest("copy-encryption and deterministic-id 1", + {$td->COMMAND => "qpdf --deterministic-id --copy-encryption=missing.pdf"}, + {$td->REGEXP => ".*the deterministic-id option is incompatible with encrypted output files", + $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); +$td->runtest("copy-encryption and deterministic-id 2", + {$td->COMMAND => "qpdf --copy-encryption=missing.pdf --deterministic-id"}, + {$td->REGEXP => ".*the deterministic-id option is incompatible with encrypted output files", + $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); # Disallow mixing positional and flag-style encryption arguments. my @bad_enc = ( diff --git a/qpdf/qtest/deterministic-id.test b/qpdf/qtest/deterministic-id.test index a975806..7e93f31 100644 --- a/qpdf/qtest/deterministic-id.test +++ b/qpdf/qtest/deterministic-id.test @@ -54,11 +54,10 @@ foreach my $d ('nn', 'ny', 'yn', 'yy') $td->runtest("deterministic ID with encryption", {$td->COMMAND => "qpdf -deterministic-id encrypted-with-images.pdf a.pdf"}, - {$td->STRING => "qpdf: INTERNAL ERROR: QPDFWriter::generateID" . - " has no data for deterministic ID." . - " This may happen if deterministic ID and" . - " file encryption are requested together.\n", - $td->EXIT_STATUS => 2}, + {$td->STRING => "qpdf: QPDFWriter: unable to generated a deterministic ID " . + "because the file to be written is encrypted (even though the file " . + "may not require a password)\n", + $td->EXIT_STATUS => 2}, $td->NORMALIZE_NEWLINES); $td->runtest("deterministic ID (C API)", {$td->COMMAND => -- libgit2 0.21.4