From ac51b29777e4f85525a2f2d2ad825bfba12075fe Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Tue, 28 Jul 2026 19:51:34 +0200 Subject: [PATCH] Avoid cycles when walking some trees --- src/core/catalog.js | 27 +++++- src/core/document.js | 5 +- test/unit/catalog_spec.js | 189 +++++++++++++++++++++++++++++++++++++ test/unit/clitests.json | 1 + test/unit/document_spec.js | 37 +++++++- test/unit/jasmine-boot.js | 1 + test/unit/test_utils.js | 23 +++++ 7 files changed, 280 insertions(+), 3 deletions(-) create mode 100644 test/unit/catalog_spec.js diff --git a/src/core/catalog.js b/src/core/catalog.js index 368725e95..0f6098c05 100644 --- a/src/core/catalog.js +++ b/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 diff --git a/src/core/document.js b/src/core/document.js index f5d72614c..6e41f3085 100644 --- a/src/core/document.js +++ b/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)) { diff --git a/test/unit/catalog_spec.js b/test/unit/catalog_spec.js new file mode 100644 index 000000000..9df3fe58d --- /dev/null +++ b/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, + ]); + }); + }); +}); diff --git a/test/unit/clitests.json b/test/unit/clitests.json index f47635abc..eb9a782e3 100644 --- a/test/unit/clitests.json +++ b/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", diff --git a/test/unit/document_spec.js b/test/unit/document_spec.js index a4718fe3f..2b6c1b176 100644 --- a/test/unit/document_spec.js +++ b/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(); diff --git a/test/unit/jasmine-boot.js b/test/unit/jasmine-boot.js index 836a9b52d..e2e3e78d5 100644 --- a/test/unit/jasmine-boot.js +++ b/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", diff --git a/test/unit/test_utils.js b/test/unit/test_utils.js index 2249a056e..619a9c212 100644 --- a/test/unit/test_utils.js +++ b/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,