diff --git a/CHANGELOG.md b/CHANGELOG.md index ca37d1cb4..fcf32f356 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- PDF image dimensions and sample parameters are checked before narrowing; an + image with an invalid one is skipped. Inline images allocate only as data is + read. + - JPEG 2000 images ignore the PDF Decode array as required. Raster decoding rejects invalid sample depths, including non-1-bit stencil masks. diff --git a/src/odr/internal/pdf/pdf_document_parser.cpp b/src/odr/internal/pdf/pdf_document_parser.cpp index 950442c41..fede55a31 100644 --- a/src/odr/internal/pdf/pdf_document_parser.cpp +++ b/src/odr/internal/pdf/pdf_document_parser.cpp @@ -721,10 +721,10 @@ ColorSpaceContext make_color_space_context(DocumentParser &parser, /// indirect reference, defaulting to `fallback`. std::int32_t image_int(DocumentParser &parser, const Dictionary &dictionary, const std::string &key, const std::int32_t fallback) { - return static_cast( - parser.resolve_object_copy(dictionary.get(key)) - .as_integer_opt() - .value_or(fallback)); + // an invalid value reads as 0, which the image checks then skip + return image_integer(parser.resolve_object_copy(dictionary.get(key)), + fallback) + .value_or(0); } /// The `/Decode` array of an image dictionary as doubles ([] when absent). @@ -827,6 +827,9 @@ DecodeOptions jbig2_decode_options(DocumentParser &parser, /// undecodable codec leaves `stencil_mask` false, so `Do` skips it. void parse_stencil_mask(DocumentParser &parser, const Dictionary &dictionary, const IndirectObject &object, XObject &x_object) { + if (image_int(parser, dictionary, "BitsPerComponent", 1) != 1) { + return; // a stencil mask has one bit per sample (8.9.6.2) + } Object filter; if (dictionary.has_key("Filter")) { filter = parser.deep_resolve_object_copy(dictionary["Filter"]); diff --git a/src/odr/internal/pdf/pdf_graphics_operator_parser.cpp b/src/odr/internal/pdf/pdf_graphics_operator_parser.cpp index 428927b1c..4273249c1 100644 --- a/src/odr/internal/pdf/pdf_graphics_operator_parser.cpp +++ b/src/odr/internal/pdf/pdf_graphics_operator_parser.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include @@ -156,7 +157,13 @@ inline_image_raw_length(const Dictionary &dictionary) { .value_or(false)) { // A stencil mask is one bit per sample (8.9.6.2). components = 1; - bits_per_component = 1; + bits_per_component = + image_integer(inline_image_entry(dictionary, "BPC", "BitsPerComponent"), + 1) + .value_or(0); + if (bits_per_component != 1) { + return std::nullopt; + } } else { const std::optional resolved = inline_color_components( inline_image_entry(dictionary, "CS", "ColorSpace")); @@ -164,26 +171,32 @@ inline_image_raw_length(const Dictionary &dictionary) { return std::nullopt; } components = *resolved; - bits_per_component = static_cast( - inline_image_entry(dictionary, "BPC", "BitsPerComponent") - .as_integer_opt() - .value_or(8)); + bits_per_component = + image_integer(inline_image_entry(dictionary, "BPC", "BitsPerComponent"), + 8) + .value_or(0); } - const std::int64_t width = - inline_image_entry(dictionary, "W", "Width").as_integer_opt().value_or(0); - const std::int64_t height = inline_image_entry(dictionary, "H", "Height") - .as_integer_opt() - .value_or(0); - if (width <= 0 || height <= 0 || bits_per_component <= 0) { + const std::int32_t width = + image_integer(inline_image_entry(dictionary, "W", "Width"), 0) + .value_or(0); + const std::int32_t height = + image_integer(inline_image_entry(dictionary, "H", "Height"), 0) + .value_or(0); + if (width <= 0 || height <= 0 || !valid_image_bit_depth(bits_per_component)) { return std::nullopt; } - // Each sample row is padded to a byte boundary (8.9.5.2). - const std::size_t bits_per_row = - static_cast(width) * components * bits_per_component; - const std::size_t bytes_per_row = (bits_per_row + 7) / 8; - return bytes_per_row * static_cast(height); + // Sample rows are byte-aligned (8.9.5.2). + const auto bits_per_row = + static_cast(width) * components * bits_per_component; + const auto bytes_per_row = (bits_per_row + 7) / 8; + if (bytes_per_row > + std::string{}.max_size() / static_cast(height)) { + return std::nullopt; + } + return static_cast(bytes_per_row) * + static_cast(height); } } // namespace @@ -302,7 +315,6 @@ GraphicsOperatorParser::read_inline_image_data(const Dictionary &dictionary) { if (const std::optional length = inline_image_raw_length(dictionary)) { std::string data; - data.reserve(*length); for (std::size_t i = 0; i < *length; ++i) { const int_type c = m_parser.geti(); if (c == eof) { diff --git a/src/odr/internal/pdf/pdf_image.cpp b/src/odr/internal/pdf/pdf_image.cpp index 215ad8924..f5377d691 100644 --- a/src/odr/internal/pdf/pdf_image.cpp +++ b/src/odr/internal/pdf/pdf_image.cpp @@ -23,11 +23,6 @@ namespace odr::internal::pdf { namespace { -bool valid_bit_depth(const std::int32_t bits) { - return bits > 0 && bits <= 16 && - std::has_single_bit(static_cast(bits)); -} - /// Reads fixed-width big-endian sample values out of a byte buffer, MSB first. /// Constructed at a row offset; rows are byte-aligned (8.9.5.2). Reads past the /// end yield zero (lenient for a truncated stream). @@ -139,6 +134,22 @@ encode_jpx(const std::string &data, const ColorSpaceDef *color_space, namespace odr::internal { +std::optional pdf::image_integer(const Object &value, + const std::int32_t fallback) { + if (value.is_null()) { + return fallback; + } + if (!value.is_integer() || !std::in_range(value.as_integer())) { + return std::nullopt; + } + return static_cast(value.as_integer()); +} + +bool pdf::valid_image_bit_depth(const std::int32_t bits) { + return bits > 0 && bits <= 16 && + std::has_single_bit(static_cast(bits)); +} + std::string pdf::encode_image_png(const std::string &samples, const std::int32_t width, const std::int32_t height, @@ -149,7 +160,7 @@ std::string pdf::encode_image_png(const std::string &samples, const std::span color_key) { const std::int32_t components = color_space.components; if (width <= 0 || height <= 0 || components <= 0 || - !valid_bit_depth(bits_per_component)) { + !valid_image_bit_depth(bits_per_component)) { return {}; } @@ -267,7 +278,7 @@ std::vector pdf::decode_mask_alpha( const std::span decode, const bool stencil, const std::int32_t base_width, const std::int32_t base_height) { if (width <= 0 || height <= 0 || base_width <= 0 || base_height <= 0 || - !valid_bit_depth(bits_per_component) || + !valid_image_bit_depth(bits_per_component) || (stencil && bits_per_component != 1)) { return {}; } diff --git a/src/odr/internal/pdf/pdf_image.hpp b/src/odr/internal/pdf/pdf_image.hpp index d2ee32f39..5d84d9f95 100644 --- a/src/odr/internal/pdf/pdf_image.hpp +++ b/src/odr/internal/pdf/pdf_image.hpp @@ -14,6 +14,14 @@ namespace odr::internal::pdf { class Object; struct ColorSpaceDef; +/// A signed 32-bit image parameter: @p fallback where it is absent, nothing +/// where it is no integer or does not fit. +std::optional image_integer(const Object &value, + std::int32_t fallback); + +/// Image sample depths allowed by ISO 32000-1 Table 89. +bool valid_image_bit_depth(std::int32_t bits); + /// Browser-ready image bytes and the format naming them (`image/jpeg` or /// `image/png`). struct EncodedImage { diff --git a/src/odr/internal/pdf/pdf_page_extractor.cpp b/src/odr/internal/pdf/pdf_page_extractor.cpp index 218ed441c..1ebdd4af3 100644 --- a/src/odr/internal/pdf/pdf_page_extractor.cpp +++ b/src/odr/internal/pdf/pdf_page_extractor.cpp @@ -547,16 +547,19 @@ void emit_inline_image(const GraphicsOperator &op, const Resources &resources, const Dictionary dictionary = normalize_inline_image(op.arguments[0].as_dictionary()); - const auto width = static_cast( - dictionary.get("Width").as_integer_opt().value_or(0)); - const auto height = static_cast( - dictionary.get("Height").as_integer_opt().value_or(0)); + const std::int32_t width = + image_integer(dictionary.get("Width"), 0).value_or(0); + const std::int32_t height = + image_integer(dictionary.get("Height"), 0).value_or(0); const std::vector decode_array = dictionary.get("Decode").as_reals(); // An inline `/ImageMask true` stencil: decode the 1-bpc bitmap and paint it // in the current fill colour, as for a stencil image XObject (ISO // 32000-1 8.9.7). if (dictionary.get("ImageMask").as_bool_opt().value_or(false)) { + if (image_integer(dictionary.get("BitsPerComponent"), 1) != 1) { + return; // a stencil mask has one bit per sample (8.9.6.2) + } DecodeResult mask = decode(dictionary.get("Filter"), dictionary.get("DecodeParms"), op.arguments[1].as_string()); @@ -583,8 +586,8 @@ void emit_inline_image(const GraphicsOperator &op, const Resources &resources, const std::shared_ptr color_space = resolve_inline_color_space(dictionary.get("ColorSpace"), resources); - const auto bits_per_component = static_cast( - dictionary.get("BitsPerComponent").as_integer_opt().value_or(8)); + const std::int32_t bits_per_component = + image_integer(dictionary.get("BitsPerComponent"), 8).value_or(0); std::optional encoded = encode_image(op.arguments[1].as_string(), dictionary.get("Filter"), diff --git a/test/src/internal/pdf/pdf_document_parser.cpp b/test/src/internal/pdf/pdf_document_parser.cpp index 2c9a15432..815e9593d 100644 --- a/test/src/internal/pdf/pdf_document_parser.cpp +++ b/test/src/internal/pdf/pdf_document_parser.cpp @@ -326,7 +326,8 @@ std::string shared_form_xobject_mini_pdf() { /// A mini-PDF whose page defines a named colour space `/CS0` in its /// `/Resources /ColorSpace` and an image XObject `Im0` referencing it by name. /// The image is a 2x2 RGB raster carried as ASCIIHex so the source stays text. -std::string named_colorspace_image_mini_pdf() { +std::string named_colorspace_image_mini_pdf( + const std::string ¶meters = "/Width 2 /Height 2 /BitsPerComponent 8") { PdfFileBuilder builder; builder.object("<< /Type /Catalog /Pages 2 0 R >>") .object("<< /Type /Pages /Kids [3 0 R] /Count 1 >>") @@ -335,9 +336,9 @@ std::string named_colorspace_image_mini_pdf() { "/ColorSpace << /CS0 [/CalRGB << /WhitePoint [1 1 1] >>] >> >> " "/Contents 4 0 R >>") .stream_object("", "/Im0 Do") - .stream_object("/Type /XObject /Subtype /Image /Width 2 /Height 2 " - "/BitsPerComponent 8 /ColorSpace /CS0 " - "/Filter /ASCIIHexDecode", + .stream_object("/Type /XObject /Subtype /Image /ColorSpace /CS0 " + "/Filter /ASCIIHexDecode " + + parameters, "FF000000FF000000FFFFFFFF>"); return builder.trailer("/Root 1 0 R").build_classic(); } @@ -878,3 +879,31 @@ TEST(DocumentParser, recovery_keeps_the_latest_compressed_copy) { } } } + +TEST(DocumentParser, skips_images_with_invalid_parameters) { + for (const std::string parameters : + {"/Width 4294967298 /Height 2 /BitsPerComponent 8", + "/Width 2 /Height -4294967294 /BitsPerComponent 8", + "/Width 2 /Height 2 /BitsPerComponent 4294967304", + "/Width 2 /Height 2 /BitsPerComponent 8 /ImageMask true"}) { + SCOPED_TRACE(parameters); + DocumentParser parser(std::make_unique( + named_colorspace_image_mini_pdf(parameters))); + const std::unique_ptr document = parser.parse_document(); + const XObject *image = first_page(*document)->resources->x_object.at("Im0"); + ASSERT_NE(image, nullptr); + EXPECT_TRUE(image->image_data.empty()); + EXPECT_FALSE(image->stencil_mask); + } +} + +// An invalid optional entry reads as its default, so the image stays. +TEST(DocumentParser, ignores_an_invalid_smask_in_data) { + DocumentParser parser( + std::make_unique(named_colorspace_image_mini_pdf( + "/Width 2 /Height 2 /BitsPerComponent 8 /SMaskInData 4294967296"))); + const std::unique_ptr document = parser.parse_document(); + const XObject *image = first_page(*document)->resources->x_object.at("Im0"); + ASSERT_NE(image, nullptr); + EXPECT_FALSE(image->image_data.empty()); +} diff --git a/test/src/internal/pdf/pdf_page_extractor.cpp b/test/src/internal/pdf/pdf_page_extractor.cpp index e9bf6995a..09be13756 100644 --- a/test/src/internal/pdf/pdf_page_extractor.cpp +++ b/test/src/internal/pdf/pdf_page_extractor.cpp @@ -1618,3 +1618,19 @@ TEST(PdfPageExtractor, form_preserves_enclosing_marked_content) { EXPECT_TRUE(texts[1].text.empty()); EXPECT_EQ(texts[2].text, "tail"); } + +TEST(PdfPageExtractor, skips_inline_images_with_invalid_parameters) { + const Resources resources; + for (const std::string parameters : + {"/W 4294967297 /H 1 /BPC 8", "/W 1 /H -4294967295 /BPC 8", + "/W 1 /H 1 /BPC 4294967304", "/W 1 /H 1 /BPC 8 /IM true"}) { + const std::string content = + "BI /CS /G /F /AHx " + parameters + " ID FF> EI"; + SCOPED_TRACE(parameters); + EXPECT_TRUE(extract_page(content, resources, Logger::null()).empty()); + } + EXPECT_TRUE(extract_page("BI /W 2147483647 /H 2147483647 /BPC 16 " + "/CS /CMYK ID x EI", + resources, Logger::null()) + .empty()); +}