fix: validate PDF color-space recursion and dimensions

CHANGELOG.md

@@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- PDF color-space parsing bounds recursive definitions and rejects invalid
+  component counts and palette ranges. Large finite palette indices clip
+  correctly.
+
 - Document path saves preserve the destination on write failure and retain
   lazy resources when saving repeatedly over the input. Existing permissions
   are preserved; temporary copies are private to their owner.

src/odr/internal/pdf/AGENTS.md

@@ -164,6 +164,11 @@ above. So each run is raised by one font ascent:
   and 64 nested calls. Invalid operands or results use the existing zero-result
   fallback, before range clipping. ISO 32000-1 7.10.5 requires at least 100
   stack entries; the larger bound accommodates ordinary tint transforms.
+- Colour-space parsing carries a 64-level bound through arrays and named
+  resource aliases. Invalid ICC component counts, an Indexed `/HiVal` outside
+  0 to 255 and unresolved base spaces are rejected before image allocation.
+  An ICC alternate that cannot stand in is dropped for the device space `N`
+  names, and a short palette reads its missing bytes as 0.
 - CMYK is naive (no ICC) and overprint is ignored. CIE, ICCBased, Indexed,
   Separation, DeviceN and Lab resolve to RGB at emission by sampling the tint
   `/Function` (types 0, 2, 3, 4).

src/odr/internal/pdf/pdf_color.cpp

@@ -5,6 +5,7 @@
 
 #include <algorithm>
 #include <cmath>
+#include <limits>
 #include <stdexcept>
 
 namespace odr::internal::pdf {
@@ -60,7 +61,8 @@ std::vector<double> read_numbers(const Object &object) {
 /// Resolve a colour-space name to a device/pattern space, or a resource space
 /// via `context.named`.
 std::shared_ptr<ColorSpaceDef> space_from_name(const std::string &name,
-                                               const ColorSpaceContext &ctx) {
+                                               const ColorSpaceContext &ctx,
+                                               const std::uint32_t depth) {
   if (name == "DeviceGray" || name == "G") {
     return device_space(ColorSpaceKind::device_gray, 1);
   }
@@ -74,7 +76,7 @@ std::shared_ptr<ColorSpaceDef> space_from_name(const std::string &name,
     return device_space(ColorSpaceKind::pattern, 1);
   }
   if (ctx.named) {
-    return ctx.named(name);
+    return ctx.named(name, depth + 1);
   }
   return nullptr;
 }
@@ -118,11 +120,11 @@ ColorSpaceDef::to_rgb(const std::span<const double> c) const {
       return {0, 0, 0};
     }
     const std::int32_t n = base->components;
-    const auto index = static_cast<std::int32_t>(std::lround(at(0)));
-    // `std::clamp` is undefined for hi < lo, so a negative `/HiVal` (malformed)
-    // must not reach it.
-    const auto offset = static_cast<std::size_t>(std::clamp<std::int32_t>(
-                            index, 0, std::max(hival, 0))) *
+    if (n <= 0 || hival < 0 || hival > 255 || !std::isfinite(at(0))) {
+      return {0, 0, 0};
+    }
+    const double index = std::clamp(at(0), 0.0, static_cast<double>(hival));
+    const auto offset = static_cast<std::size_t>(std::lround(index)) *
                         static_cast<std::size_t>(n);
     std::vector<double> base_components(static_cast<std::size_t>(n), 0.0);
     for (std::int32_t j = 0; j < n; ++j) {
@@ -219,11 +221,15 @@ std::array<double, 3> pdf::cmyk_to_rgb(const double c, const double m,
 }
 
 std::shared_ptr<pdf::ColorSpaceDef>
-pdf::parse_color_space(const Object &object, const ColorSpaceContext &context) {
+pdf::parse_color_space(const Object &object, const ColorSpaceContext &context,
+                       const std::uint32_t depth) {
+  if (depth >= 64) {
+    return nullptr;
+  }
   const Object resolved = context.resolve(object);
 
   if (resolved.is_name()) {
-    return space_from_name(resolved.as_name(), context);
+    return space_from_name(resolved.as_name(), context, depth);
   }
   if (!resolved.is_array() || resolved.as_array().empty()) {
     return nullptr;
@@ -243,14 +249,25 @@ pdf::parse_color_space(const Object &object, const ColorSpaceContext &context) {
     auto def = std::make_shared<ColorSpaceDef>();
     def->kind = ColorSpaceKind::icc_based;
     const Object stream_dict = context.resolve(array[1]);
-    if (stream_dict.is_dictionary()) {
-      const Dictionary &dict = stream_dict.as_dictionary();
-      def->components =
-          dict.get("N").is_integer()
-              ? static_cast<std::int32_t>(dict.get("N").as_integer())
-              : 3;
-      if (dict.has_value("Alternate")) {
-        def->alternate = parse_color_space(dict.get("Alternate"), context);
+    if (!stream_dict.is_dictionary()) {
+      return nullptr;
+    }
+    const Dictionary &dict = stream_dict.as_dictionary();
+    const Object count = context.resolve(dict.get("N"));
+    if (!count.is_integer() ||
+        (count.as_integer() != 1 && count.as_integer() != 3 &&
+         count.as_integer() != 4)) {
+      return nullptr;
+    }
+    def->components = static_cast<std::int32_t>(count.as_integer());
+    if (dict.has_value("Alternate")) {
+      def->alternate =
+          parse_color_space(dict.get("Alternate"), context, depth + 1);
+      // an alternate that cannot stand in leaves the device space `N` names
+      if (def->alternate != nullptr &&
+          (def->alternate->kind == ColorSpaceKind::pattern ||
+           def->alternate->components != def->components)) {
+        def->alternate = nullptr;
       }
     }
     return def;
@@ -288,48 +305,66 @@ pdf::parse_color_space(const Object &object, const ColorSpaceContext &context) {
     auto def = std::make_shared<ColorSpaceDef>();
     def->kind = ColorSpaceKind::indexed;
     def->components = 1;
-    def->base = parse_color_space(array[1], context);
-    def->hival =
-        static_cast<std::int32_t>(context.resolve(array[2]).as_integer());
+    def->base = parse_color_space(array[1], context, depth + 1);
+    const Object high = context.resolve(array[2]);
+    if (def->base == nullptr || def->base->kind == ColorSpaceKind::indexed ||
+        def->base->kind == ColorSpaceKind::pattern || !high.is_integer() ||
+        high.as_integer() < 0 || high.as_integer() > 255) {
+      return nullptr;
+    }
+    def->hival = static_cast<std::int32_t>(high.as_integer());
     const Object lookup = context.resolve(array[3]);
     if (lookup.is_string()) {
       def->lookup = lookup.as_string();
     } else {
       def->lookup = context.load_stream(array[3]);
     }
+    // a short palette reads its missing bytes as 0
     return def;
   }
-  if (family == "Separation") {
+  if (family == "Separation" || family == "DeviceN") {
     if (array.size() < 4) {
       return nullptr;
     }
     auto def = std::make_shared<ColorSpaceDef>();
-    def->kind = ColorSpaceKind::separation;
-    def->components = 1;
-    def->alternate = parse_color_space(array[2], context);
-    def->tint = parse_function(
-        array[3], FunctionContext{context.resolve, context.load_stream});
-    return def;
-  }
-  if (family == "DeviceN") {
-    if (array.size() < 4) {
+    const Object names = context.resolve(array[1]);
+    if (family == "Separation") {
+      if (!names.is_name()) {
+        return nullptr;
+      }
+      def->kind = ColorSpaceKind::separation;
+      def->components = 1;
+    } else {
+      if (!names.is_array() || names.as_array().empty() ||
+          names.as_array().size() >
+              static_cast<std::size_t>(
+                  std::numeric_limits<std::int32_t>::max())) {
+        return nullptr;
+      }
+      for (const Object &name : names.as_array()) {
+        if (!context.resolve(name).is_name()) {
+          return nullptr;
+        }
+      }
+      def->kind = ColorSpaceKind::device_n;
+      def->components = static_cast<std::int32_t>(names.as_array().size());
+    }
+    def->alternate = parse_color_space(array[2], context, depth + 1);
+    if (def->alternate == nullptr) {
       return nullptr;
     }
-    auto def = std::make_shared<ColorSpaceDef>();
-    def->kind = ColorSpaceKind::device_n;
-    const Object names = context.resolve(array[1]);
-    def->components = names.is_array()
-                          ? static_cast<std::int32_t>(names.as_array().size())
-                          : 1;
-    def->alternate = parse_color_space(array[2], context);
     def->tint = parse_function(
         array[3], FunctionContext{context.resolve, context.load_stream});
     return def;
   }
   if (family == "Pattern") {
     auto def = device_space(ColorSpaceKind::pattern, 1);
     if (array.size() >= 2) {
-      def->base = parse_color_space(array[1], context);
+      def->base = parse_color_space(array[1], context, depth + 1);
+      if (def->base == nullptr || def->base->kind == ColorSpaceKind::pattern) {
+        return nullptr;
+      }
+      def->components = def->base->components;
     }
     return def;
   }

src/odr/internal/pdf/pdf_color.hpp

@@ -67,11 +67,13 @@ struct ColorSpaceDef {
 /// How `parse_color_space` reaches indirect data: `resolve` dereferences,
 /// `load_stream` decodes a stream's bytes (ICC profiles, an Indexed palette
 /// given as a stream), and `named` looks up a colour space referenced by name
-/// (a base space, the `/ColorSpace` resource table).
+/// (a base space, the `/ColorSpace` resource table), forwarding its depth.
 struct ColorSpaceContext {
   std::function<Object(const Object &)> resolve;
   std::function<std::string(const Object &)> load_stream;
-  std::function<std::shared_ptr<ColorSpaceDef>(const std::string &)> named;
+  std::function<std::shared_ptr<ColorSpaceDef>(const std::string &,
+                                               std::uint32_t)>
+      named;
 };
 
 /// DeviceCMYK -> sRGB without an ICC engine: pdf.js's polynomial fit of Adobe's
@@ -80,8 +82,9 @@ std::array<double, 3> cmyk_to_rgb(double c, double m, double y, double k);
 
 /// Build a colour space from its PDF object — a name (`/DeviceRGB`, …) or an
 /// array (`[/ICCBased 5 0 R]`, `[/Separation …]`, …). Returns `nullptr` for an
-/// unsupported or malformed definition.
+/// unsupported, malformed or excessively nested definition.
 std::shared_ptr<ColorSpaceDef>
-parse_color_space(const Object &object, const ColorSpaceContext &context);
+parse_color_space(const Object &object, const ColorSpaceContext &context,
+                  std::uint32_t depth = 0);
 
 } // namespace odr::internal::pdf

src/odr/internal/pdf/pdf_document_parser.cpp

@@ -1024,7 +1024,8 @@ ColorSpaceContext make_color_space_context(DocumentParser &parser,
   bind_parser_io(context, parser);
   if (resources != nullptr) {
     context.named =
-        [resources](const std::string &name) -> std::shared_ptr<ColorSpaceDef> {
+        [resources](const std::string &name,
+                    std::uint32_t) -> std::shared_ptr<ColorSpaceDef> {
       const auto it = resources->color_space.find(name);
       return it != resources->color_space.end() ? it->second : nullptr;
     };
@@ -1173,13 +1174,15 @@ Resources *parse_resources(State &state, const Object &object) {
     // A base/alternate space may be named (referencing another `/ColorSpace`
     // entry); resolve it lazily from the same table, caching the result.
     context.named =
-        [&](const std::string &name) -> std::shared_ptr<ColorSpaceDef> {
+        [&](const std::string &name,
+            const std::uint32_t depth) -> std::shared_ptr<ColorSpaceDef> {
       if (const auto it = resources->color_space.find(name);
           it != resources->color_space.end()) {
         return it->second;
       }
       if (color_space_table.has_value(name)) {
-        auto def = parse_color_space(color_space_table.get(name), context);
+        auto def =
+            parse_color_space(color_space_table.get(name), context, depth);
         resources->color_space[name] = def;
         return def;
       }

src/odr/internal/pdf/pdf_page_extractor.cpp

@@ -527,7 +527,8 @@ resolve_inline_color_space(const Object &color_space,
   context.resolve = [](const Object &o) { return o; };
   context.load_stream = [](const Object &) { return std::string{}; };
   context.named =
-      [&resources](const std::string &name) -> std::shared_ptr<ColorSpaceDef> {
+      [&resources](const std::string &name,
+                   std::uint32_t) -> std::shared_ptr<ColorSpaceDef> {
     const auto it = resources.color_space.find(name);
     return it != resources.color_space.end() ? it->second : nullptr;
   };

test/src/internal/pdf/pdf_color.cpp

@@ -3,6 +3,8 @@
 #include <odr/internal/pdf/pdf_object.hpp>
 
 #include <array>
+#include <cstdint>
+#include <limits>
 #include <string>
 #include <vector>
 
@@ -28,7 +30,7 @@ Object reals(std::initializer_list<double> values) {
   return Object(Array(std::move(holder)));
 }
 
-ColorSpaceDef device(const ColorSpaceKind kind, const int components) {
+ColorSpaceDef device(const ColorSpaceKind kind, const std::int32_t components) {
   ColorSpaceDef def;
   def.kind = kind;
   def.components = components;
@@ -110,6 +112,12 @@ TEST(PdfColor, indexed_palette) {
   EXPECT_EQ(def->kind, ColorSpaceKind::indexed);
   EXPECT_EQ(to_rgb(*def, {0}), (std::array<double, 3>{1, 0, 0}));
   EXPECT_EQ(to_rgb(*def, {1}), (std::array<double, 3>{0, 1, 0}));
+  for (const double value : {-1e100, 0.49}) {
+    EXPECT_EQ(to_rgb(*def, {value}), (std::array<double, 3>{1, 0, 0}));
+  }
+  for (const double value : {0.5, 1e100}) {
+    EXPECT_EQ(to_rgb(*def, {value}), (std::array<double, 3>{0, 1, 0}));
+  }
 }
 
 // Separation samples its tint transform, then converts through the alternate.
@@ -178,3 +186,79 @@ TEST(PdfColor, name_resolves_device_space) {
   EXPECT_EQ(def->kind, ColorSpaceKind::device_cmyk);
   EXPECT_EQ(def->components, 4);
 }
+
+TEST(PdfColor, invalid_component_counts_and_palettes_are_rejected) {
+  for (const Integer count : {Integer{-1}, Integer{0}, Integer{2}, Integer{5},
+                              std::numeric_limits<Integer>::max()}) {
+    Dictionary profile;
+    profile["N"] = Object(count);
+    EXPECT_EQ(parse_color_space(
+                  Object(Array({Object(Name{"ICCBased"}), Object(profile)})),
+                  context()),
+              nullptr)
+        << count;
+  }
+  for (const Integer hival :
+       {Integer{-1}, Integer{256}, std::numeric_limits<Integer>::max()}) {
+    EXPECT_EQ(
+        parse_color_space(
+            Object(Array({Object(Name{"Indexed"}), Object(Name{"DeviceGray"}),
+                          Object(hival),
+                          Object(StandardString(std::string(256, '\0')))})),
+            context()),
+        nullptr)
+        << hival;
+  }
+}
+
+// A palette shorter than `/HiVal` asks for keeps its entries; the missing
+// bytes read as 0.
+TEST(PdfColor, a_short_palette_reads_missing_bytes_as_zero) {
+  const auto def = parse_color_space(
+      Object(Array({Object(Name{"Indexed"}), Object(Name{"DeviceRGB"}),
+                    Object(Integer{1}),
+                    Object(StandardString(std::string("\xff\0\0\0\xff", 5)))})),
+      context());
+  ASSERT_NE(def, nullptr);
+  EXPECT_EQ(to_rgb(*def, {0}), (std::array<double, 3>{1, 0, 0}));
+  EXPECT_EQ(to_rgb(*def, {1}), (std::array<double, 3>{0, 1, 0}));
+}
+
+TEST(PdfColor, device_n_requires_nonempty_colorant_names) {
+  for (const Object &names : {Object(Name{"Spot"}), Object(Array{}),
+                              Object(Array({Object(Integer{1})}))}) {
+    EXPECT_EQ(
+        parse_color_space(Object(Array({Object(Name{"DeviceN"}), names,
+                                        Object(Name{"DeviceRGB"}), Object{}})),
+                          context()),
+        nullptr);
+  }
+}
+
+TEST(PdfColor, recursive_alternates_stop_at_the_depth_limit) {
+  Dictionary profile;
+  profile["N"] = Object(Integer{3});
+  profile["Alternate"] = Object(ObjectReference{1, 0});
+  const Object cyclic(Array({Object(Name{"ICCBased"}), Object(profile)}));
+  ColorSpaceContext ctx = context();
+  ctx.resolve = [&](const Object &object) {
+    return object.is_reference() ? cyclic : object;
+  };
+  const auto def = parse_color_space(cyclic, ctx);
+  ASSERT_NE(def, nullptr);
+  EXPECT_EQ(def->components, 3);
+  ASSERT_NE(parse_color_space(Object(Name{"DeviceRGB"}), ctx), nullptr);
+}
+
+// An alternate with another component count cannot stand in, so the space
+// falls back to the device space `N` names.
+TEST(PdfColor, a_mismatched_icc_alternate_is_ignored) {
+  Dictionary profile;
+  profile["N"] = Object(Integer{3});
+  profile["Alternate"] = Object(Name{"DeviceGray"});
+  const auto def = parse_color_space(
+      Object(Array({Object(Name{"ICCBased"}), Object(profile)})), context());
+  ASSERT_NE(def, nullptr);
+  EXPECT_EQ(def->alternate, nullptr);
+  EXPECT_EQ(to_rgb(*def, {1, 0, 0}), (std::array<double, 3>{1, 0, 0}));
+}