Avoid cycles when walking some trees

src/core/catalog.js

@@ -1598,6 +1598,9 @@ class Catalog {
     const xref = this.xref;
     let total = 0,
       ref = pageRef;
+    // Prevent circular references in the /Pages tree.
+    const visited = new RefSet();
+    visited.put(pageRef);
 
     while (true) {
       const node = await xref.fetchAsync(ref);
@@ -1617,6 +1620,12 @@ class Catalog {
         throw new FormatError("Node must be a dictionary.");
       }
       const parentRef = node.getRaw("Parent");
+      if (parentRef instanceof Ref) {
+        if (visited.has(parentRef)) {
+          throw new FormatError("Pages tree contains circular reference.");
+        }
+        visited.put(parentRef);
+      }
 
       const parent = await node.getAsync("Parent");
       if (!parent) {
@@ -1729,9 +1738,19 @@ class Catalog {
     // reached (e.g. integer MCIDs or MCR/OBJR dicts without further K).
     if (!pageRef) {
       const queue = [seDict];
+      // Prevent circular references in the structure tree.
+      const visited = new RefSet();
+      visited.put(seRef);
       while (queue.length > 0 && !pageRef) {
         const node = queue.shift();
-        const kids = node.get("K");
+        let kids = node.getRaw("K");
+        if (kids instanceof Ref) {
+          if (visited.has(kids)) {
+            continue;
+          }
+          visited.put(kids);
+          kids = xref.fetch(kids);
+        }
         let kidsArr;
         if (Array.isArray(kids)) {
           kidsArr = kids;
@@ -1741,6 +1760,12 @@ class Catalog {
           continue;
         }
         for (const kid of kidsArr) {
+          if (kid instanceof Ref) {
+            if (visited.has(kid)) {
+              continue;
+            }
+            visited.put(kid);
+          }
           const kidObj = xref.fetchIfRef(kid);
           if (!(kidObj instanceof Dict)) {
             continue; // integer MCID – leaf node, no Pg here

src/core/document.js

@@ -1880,12 +1880,15 @@ class PDFDocument {
       name = name === "" ? partName : `${name}.${partName}`;
     } else {
       let obj = field;
+      // The `Parent` chain can be cyclic, hence the local `RefSet`.
+      const walkedRefs = new RefSet();
       while (true) {
         obj = obj.getRaw("Parent") || parentRef;
         if (obj instanceof Ref) {
-          if (visitedRefs.has(obj)) {
+          if (visitedRefs.has(obj) || walkedRefs.has(obj)) {
             break;
           }
+          walkedRefs.put(obj);
           obj = await xref.fetchAsync(obj);
         }
         if (!(obj instanceof Dict)) {

test/unit/catalog_spec.js

@@ -0,0 +1,189 @@
+/* Copyright 2026 Mozilla Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Dict, Name, Ref } from "../../src/core/primitives.js";
+import { BoundedXRefMock } from "./test_utils.js";
+import { Catalog } from "../../src/core/catalog.js";
+import { FormatError } from "../../src/shared/util.js";
+
+describe("Catalog", function () {
+  class CatalogXRefMock extends BoundedXRefMock {
+    #catalogObj = null;
+
+    constructor(entries, catalogObj) {
+      super(entries);
+      this.#catalogObj = catalogObj;
+
+      for (const { data } of entries) {
+        data.assignXref(this);
+      }
+      catalogObj?.assignXref(this);
+    }
+
+    getCatalogObj() {
+      return this.#catalogObj;
+    }
+  }
+
+  function makeCatalog(entries) {
+    const topPagesDict = new Dict();
+    topPagesDict.set("Type", Name.get("Pages"));
+    topPagesDict.set("Kids", []);
+    topPagesDict.set("Count", 0);
+
+    const catDict = new Dict();
+    catDict.set("Type", Name.get("Catalog"));
+    catDict.set("Pages", topPagesDict);
+
+    const xref = new CatalogXRefMock(entries, catDict);
+    return new Catalog(/* pdfManager = */ null, xref);
+  }
+
+  describe("getPageIndex", function () {
+    it("should reject a circular `Parent` chain", async function () {
+      // The page and its parent reference each other, and both `Kids` arrays
+      // are consistent with that, hence walking up the tree never terminates.
+      const pageRef = Ref.get(4, 0);
+      const pagesRef = Ref.get(5, 0);
+
+      const pageDict = new Dict();
+      pageDict.set("Type", Name.get("Page"));
+      pageDict.set("Parent", pagesRef);
+      pageDict.set("Kids", [pagesRef]);
+
+      const pagesDict = new Dict();
+      pagesDict.set("Type", Name.get("Pages"));
+      pagesDict.set("Parent", pageRef);
+      pagesDict.set("Kids", [pageRef]);
+      pagesDict.set("Count", 1);
+
+      const catalog = makeCatalog([
+        { ref: pageRef, data: pageDict },
+        { ref: pagesRef, data: pagesDict },
+      ]);
+
+      await expectAsync(catalog.getPageIndex(pageRef)).toBeRejectedWithError(
+        FormatError,
+        "Pages tree contains circular reference."
+      );
+    });
+  });
+
+  describe("parseDestDictionary", function () {
+    function makeStructElements(entries) {
+      const xref = new BoundedXRefMock(entries);
+      for (const { data } of entries) {
+        data.assignXref(xref);
+      }
+      return xref;
+    }
+
+    it("should handle a circular `K` chain in the `SE` entry", function () {
+      const seRef = Ref.get(5, 0);
+      const kidRef = Ref.get(6, 0);
+
+      // Neither element provides a `Pg` entry, hence the descendants of the
+      // structure element are all visited before giving up.
+      const seDict = new Dict();
+      seDict.set("S", Name.get("Sect"));
+      seDict.set("K", [kidRef]);
+
+      const kidDict = new Dict();
+      kidDict.set("S", Name.get("Sect"));
+      kidDict.set("K", [seRef]);
+
+      const xref = makeStructElements([
+        { ref: seRef, data: seDict },
+        { ref: kidRef, data: kidDict },
+      ]);
+
+      const destDict = new Dict(xref);
+      destDict.set("SE", seRef);
+
+      const resultObj = {};
+      Catalog.parseDestDictionary({ destDict, resultObj });
+      expect(resultObj.dest).toBeUndefined();
+      // `parseDestDictionary` swallows errors from the `SE` parsing, hence the
+      // number of fetches is what actually proves that the cycle was detected.
+      expect(xref.fetchCount).toBeLessThan(10);
+    });
+
+    it("should handle a circular single-reference `K` chain", function () {
+      const seRef = Ref.get(5, 0);
+      const kidRef = Ref.get(6, 0);
+
+      const seDict = new Dict();
+      seDict.set("S", Name.get("Sect"));
+      seDict.set("K", kidRef);
+
+      const kidDict = new Dict();
+      kidDict.set("S", Name.get("Sect"));
+      kidDict.set("K", seRef);
+
+      const xref = makeStructElements([
+        { ref: seRef, data: seDict },
+        { ref: kidRef, data: kidDict },
+      ]);
+
+      const destDict = new Dict(xref);
+      destDict.set("SE", seRef);
+
+      const resultObj = {};
+      Catalog.parseDestDictionary({ destDict, resultObj });
+      expect(resultObj.dest).toBeUndefined();
+      expect(xref.fetchCount).toBeLessThan(10);
+    });
+
+    it("should find the page of a nested `SE` entry", function () {
+      const seRef = Ref.get(5, 0);
+      const kidRef = Ref.get(6, 0);
+      const grandKidRef = Ref.get(7, 0);
+      const pageRef = Ref.get(8, 0);
+
+      const seDict = new Dict();
+      seDict.set("S", Name.get("Sect"));
+      seDict.set("K", [kidRef]);
+
+      // Only the grand-kid provides a `Pg` entry, hence the descendants must
+      // actually be enqueued and visited to find it.
+      const kidDict = new Dict();
+      kidDict.set("S", Name.get("Sect"));
+      kidDict.set("K", [grandKidRef]);
+
+      const grandKidDict = new Dict();
+      grandKidDict.set("S", Name.get("P"));
+      grandKidDict.set("Pg", pageRef);
+
+      const xref = makeStructElements([
+        { ref: seRef, data: seDict },
+        { ref: kidRef, data: kidDict },
+        { ref: grandKidRef, data: grandKidDict },
+      ]);
+
+      const destDict = new Dict(xref);
+      destDict.set("SE", seRef);
+
+      const resultObj = {};
+      Catalog.parseDestDictionary({ destDict, resultObj });
+      expect(resultObj.dest).toEqual([
+        pageRef,
+        { name: "XYZ" },
+        null,
+        null,
+        null,
+      ]);
+    });
+  });
+});

test/unit/clitests.json

@@ -11,6 +11,7 @@
     "autolinker_spec.js",
     "bidi_spec.js",
     "canvas_factory_spec.js",
+    "catalog_spec.js",
     "cff_parser_spec.js",
     "cmap_spec.js",
     "colorspace_spec.js",

test/unit/document_spec.js

@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { createIdFactory, XRefMock } from "./test_utils.js";
+import { BoundedXRefMock, createIdFactory, XRefMock } from "./test_utils.js";
 import { Dict, Name, Ref } from "../../src/core/primitives.js";
 import { PDFDocument } from "../../src/core/document.js";
 import { StringStream } from "../../src/core/stream.js";
@@ -630,6 +630,41 @@ describe("document", function () {
       expect(fields.parent).toEqual(["358R"]);
     });
 
+    it("should get field objects with a circular `Parent` chain", async function () {
+      // A field without a `T` entry inherits its name from the `Parent` chain,
+      // which may be circular in corrupt/malicious documents.
+      const widgetRef = Ref.get(1, 0);
+      const parentRef = Ref.get(2, 0);
+      const grandParentRef = Ref.get(3, 0);
+
+      const widgetDict = new Dict();
+      widgetDict.set("Type", Name.get("Annot"));
+      widgetDict.set("Subtype", Name.get("Widget"));
+      widgetDict.set("FT", Name.get("Btn"));
+      widgetDict.set("Parent", parentRef);
+
+      // Note that the cycle doesn't include the field itself, and that neither
+      // ancestor provides a `T` entry.
+      const parentDict = new Dict();
+      parentDict.set("Parent", grandParentRef);
+      const grandParentDict = new Dict();
+      grandParentDict.set("Parent", parentRef);
+
+      const xref = new BoundedXRefMock([
+        { ref: widgetRef, data: widgetDict },
+        { ref: parentRef, data: parentDict },
+        { ref: grandParentRef, data: grandParentDict },
+      ]);
+
+      const acroForm = new Dict();
+      acroForm.set("Fields", [widgetRef]);
+      const pdfDocument = getDocument(acroForm, xref);
+
+      const { allFields } = await pdfDocument.fieldObjects;
+      expect(Object.keys(allFields)).toEqual([""]);
+      expect(allFields[""].map(obj => obj.id)).toEqual(["1R"]);
+    });
+
     it("should check if fields have any actions", async function () {
       const acroForm = new Dict();
 

test/unit/jasmine-boot.js

@@ -57,6 +57,7 @@ async function initializePDFJS(callback) {
       "pdfjs-test/unit/autolinker_spec.js",
       "pdfjs-test/unit/bidi_spec.js",
       "pdfjs-test/unit/canvas_factory_spec.js",
+      "pdfjs-test/unit/catalog_spec.js",
       "pdfjs-test/unit/cff_parser_spec.js",
       "pdfjs-test/unit/cmap_spec.js",
       "pdfjs-test/unit/colorspace_spec.js",

test/unit/test_utils.js

@@ -177,6 +177,28 @@ class XRefMock {
   }
 }
 
+/**
+ * `XRefMock` variant that limits the number of fetches, such that tests
+ * exercising cycle detection fail rather than hang when a guard is missing.
+ */
+class BoundedXRefMock extends XRefMock {
+  #limit;
+
+  fetchCount = 0;
+
+  constructor(array, limit = 100) {
+    super(array);
+    this.#limit = limit;
+  }
+
+  fetch(ref) {
+    if (++this.fetchCount > this.#limit) {
+      throw new Error(`BoundedXRefMock: more than ${this.#limit} fetches.`);
+    }
+    return super.fetch(ref);
+  }
+}
+
 function createIdFactory(pageIndex) {
   const pdfManager = {
     get docId() {
@@ -285,6 +307,7 @@ class TestPdfsServer {
 }
 
 export {
+  BoundedXRefMock,
   buildGetDocumentParams,
   CMAP_URL,
   createIdFactory,