From ed81227557992a3745294ad8f7b5504ffb6a87a5 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Thu, 23 Jul 2026 12:39:43 +0200 Subject: [PATCH] Improve the detection of the changes made after a document has been signed --- src/core/document.js | 104 ++++++++++++++------ src/core/xref.js | 24 +++++ test/pdfs/.gitignore | 1 + test/pdfs/signed_verified.pdf | Bin 0 -> 10252 bytes test/unit/api_spec.js | 77 +++++++++++++++ test/unit/document_spec.js | 86 +++++++++++++++- web/digital_signature_properties_manager.js | 2 + web/firefoxcom.js | 5 + web/genericcom.js | 2 + 9 files changed, 269 insertions(+), 32 deletions(-) create mode 100644 test/pdfs/signed_verified.pdf diff --git a/src/core/document.js b/src/core/document.js index 685c3e7eb3571..a12de7a99859b 100644 --- a/src/core/document.js +++ b/src/core/document.js @@ -81,6 +81,7 @@ import { XFAFactory } from "./xfa/factory.js"; import { XRef } from "./xref.js"; const LETTER_SIZE_MEDIABOX = [0, 0, 612, 792]; +const SIGNATURE_TAIL_CHUNK_SIZE = 65536; class Page { #resourcesPromise = null; @@ -1004,11 +1005,10 @@ function find(stream, signature, limit = 1024, backwards = false) { class PDFDocument { #pagePromises = new Map(); - // Map — populated by the + // Map — populated by the // `signatures` getter, consumed by `getSignatureData`. We deliberately - // keep the byte payload out of the metadata array so it doesn't ride - // the worker→main `postMessage` boundary unless the viewer actually - // asks to verify (one shot per signature). + // keep the signed byte spans out of the metadata array and only slice + // them out of the stream when the viewer actually asks to verify. #signatureData = null; #version = null; @@ -2026,12 +2026,47 @@ class PDFDocument { } } - // The /ByteRange of a signature that covers the whole document usually - // ends a few bytes before the file's actual end — the trailer, - // `startxref` offset and `%%EOF` marker are conventionally left outside - // the signed range. 100 bytes covers the worst case (large - // `startxref` offsets, optional whitespace) without false positives. - static #WHOLE_DOCUMENT_TAIL_FUZZ = 100; + async #getByteRange(begin, end) { + try { + return this.stream.getByteRange(begin, end); + } catch (ex) { + if (!(ex instanceof MissingDataException)) { + throw ex; + } + await this.pdfManager.requestRange(begin, end); + return this.#getByteRange(begin, end); + } + } + + async #coversWholeDocument(signedEnd, modificationsAfterSignature) { + if (modificationsAfterSignature > 0) { + return false; + } + + const fileLength = this.stream.end; + for ( + let begin = signedEnd; + begin < fileLength; + begin += SIGNATURE_TAIL_CHUNK_SIZE + ) { + const end = Math.min(begin + SIGNATURE_TAIL_CHUNK_SIZE, fileLength); + const tail = await this.#getByteRange(begin, end); + + for (const byte of tail) { + if ( + byte !== 0x00 && // null + byte !== 0x09 && // horizontal tab + byte !== 0x0a && // line feed + byte !== 0x0c && // form feed + byte !== 0x0d && // carriage return + byte !== 0x20 // space + ) { + return false; + } + } + } + return true; + } #parseSignatureDict(field, sigDict, fieldRef) { const byteRange = sigDict.get("ByteRange"); @@ -2075,15 +2110,12 @@ class PDFDocument { if ( a !== 0 || b <= 0 || - d < 0 || a + b > c || c + d > fileLength || fileLength === 0 ) { return null; } - const data = [stream.getByteRange(a, a + b), stream.getByteRange(c, c + d)]; - const pkcs7 = stringToBytes(contents); const t = field.get("T"); @@ -2097,13 +2129,6 @@ class PDFDocument { const refKey = fieldRef instanceof Ref ? fieldRef.toString() : "inline"; const id = `${refKey}:${a}-${b}-${c}-${d}`; - // A signature "covers the whole document" iff the gap between the - // last signed byte and EOF is no more than the conventional - // trailer-slack window. Compare on the gap (not on the absolute - // lastSignedByte) so a tiny file with a tiny ByteRange isn't - // mis-flagged as full-coverage. - const tailGap = fileLength - (c + d); - return { id, fieldName, @@ -2119,11 +2144,8 @@ class PDFDocument { signatureType, byteRange, pkcs7, - data, revisionIndex: 0, parentId: null, - coversWholeDocument: - tailGap >= 0 && tailGap <= PDFDocument.#WHOLE_DOCUMENT_TAIL_FUZZ, }; } @@ -2143,6 +2165,18 @@ class PDFDocument { const collected = []; this.#collectSignatureFields(fields, collected, new RefSet()); + await Promise.all( + collected.map(async signature => { + const signedEnd = signature.byteRange[2] + signature.byteRange[3]; + signature.modificationsAfterSignature = + this.xref.countUpdatesAfter?.(signedEnd) ?? null; + signature.coversWholeDocument = await this.#coversWholeDocument( + signedEnd, + signature.modificationsAfterSignature + ); + }) + ); + // Group sub-signatures by ByteRange containment: outer revision is // the largest covering signature (largest c + d). Sort descending, // then point each later signature at the smallest enclosing parent @@ -2165,14 +2199,14 @@ class PDFDocument { } } } - // Split bytes (`data`, `pkcs7`) out of the metadata so the array - // we ship to the main thread stays small. The viewer fetches the - // bytes on demand via `getSignatureData(id)`, one signature at a - // time, only when verification is about to run. + // Keep the PKCS#7 blob and byte-range information worker-side so the + // metadata array stays small. The viewer fetches the signed bytes on + // demand via `getSignatureData(id)`, one signature at a time, only + // when verification is about to run. const signatureData = new Map(); const metadata = collected.map(sig => { - const { data, pkcs7, ...rest } = sig; - signatureData.set(sig.id, { data, pkcs7 }); + const { pkcs7, ...rest } = sig; + signatureData.set(sig.id, { byteRange: sig.byteRange, pkcs7 }); return rest; }); this.#signatureData = signatureData; @@ -2185,7 +2219,17 @@ class PDFDocument { async getSignatureData(id) { // Ensure parsing is finished and `#signatureData` is populated. await this.signatures; - return this.#signatureData?.get(id) ?? null; + const signature = this.#signatureData?.get(id); + if (!signature) { + return null; + } + const { byteRange, pkcs7 } = signature; + const [a, b, c, d] = byteRange; + const data = await Promise.all([ + this.#getByteRange(a, a + b), + this.#getByteRange(c, c + d), + ]); + return { data, pkcs7 }; } get hasJSActions() { diff --git a/src/core/xref.js b/src/core/xref.js index e2b69a48dc608..f68a121485c5a 100644 --- a/src/core/xref.js +++ b/src/core/xref.js @@ -38,6 +38,9 @@ class XRef { this.pdfManager = pdfManager; this.entries = []; this._xrefStms = new Set(); + this._xrefSectionOffsets = new Set(); + this._xrefSectionsComplete = true; + this._parsedWithRecovery = false; this._cacheMap = new Map(); // Prepare the XRef cache. this._pendingRefs = new RefSet(); this._newPersistentRefNum = null; @@ -97,6 +100,7 @@ class XRef { } parse(recoveryMode = false) { + this._parsedWithRecovery = recoveryMode; let trailerDict; if (!recoveryMode) { trailerDict = this.readXRef(); @@ -769,6 +773,8 @@ class XRef { throw new FormatError("Invalid XRef stream header"); } + this._xrefSectionOffsets.add(startXRef); + // Recursively get previous dictionary, if any obj = dict.get("Prev"); if (Number.isInteger(obj)) { @@ -782,6 +788,7 @@ class XRef { if (e instanceof MissingDataException) { throw e; } + this._xrefSectionsComplete = false; info("(while reading XRef): " + e); } this.startXRefQueue.shift(); @@ -796,6 +803,23 @@ class XRef { throw new XRefParseException(); } + countUpdatesAfter(offset) { + if (this._parsedWithRecovery || !this._xrefSectionsComplete) { + return null; + } + const relativeOffset = offset - this.stream.start; + let count = 0; + for (const sectionOffset of this._xrefSectionOffsets) { + if ( + sectionOffset >= relativeOffset && + !this._xrefStms.has(sectionOffset) + ) { + count++; + } + } + return count; + } + getEntry(i) { const xrefEntry = this.entries[i]; if (xrefEntry && !xrefEntry.free && xrefEntry.offset) { diff --git a/test/pdfs/.gitignore b/test/pdfs/.gitignore index 803f4e5995c7e..11aa0c6bfa6fc 100644 --- a/test/pdfs/.gitignore +++ b/test/pdfs/.gitignore @@ -948,3 +948,4 @@ !saslprep-r6.pdf !jpx_smaskindata.pdf !nonisolated_blend_smask.pdf +!signed_verified.pdf diff --git a/test/pdfs/signed_verified.pdf b/test/pdfs/signed_verified.pdf new file mode 100644 index 0000000000000000000000000000000000000000..e4d30f9471d608b6e378563befada81ca3746c22 GIT binary patch literal 10252 zcmeHN*=`%j6@BMd6bu-UgGj1DtwDLw=NX?Jwl~Wp`Cp&DDko z^36B+nYHU>HCmtj)cCE<^Y6O)xY#`&`|Wxp?BKz=^5ugyq@l&uT}(f;%dQFi2l6Au ze&rOgk|UhhuSaa#5K|qH^B3)=-wq2TGX;;;+Y3hQv|)<*zd69Ms|->-TkRcn5#gq z#&!VufU`Y&{`|Y|yAdTq9Cs_t5(_tP7aMc7Y{zDC-7d-b59B00=9FuysH4;C&j{*8 z+idzZIhnVck!~@z;l;b`Vj0QoG z3{v8OW-2IW9_itNhOb(}GW_r;CrdnHCq6T8;TccEDpr}K&I#(6NhWh&Xetc$b>xW) zr@UAAD7exEe$rR`Ag66DFi2DgiRf3&WR~xQ3^BJC+(OIETJI6+rYhKIeTM3G31?WLROjv z9s-3gESnS@2+NFfG%#fxG6h#cRG9`Vk&Cy&Bw?)-xo`t)tWv>fjmW*plsn3c@FpN3 z^Z-X-Eb8ctPfly5LI4w;W?-yQ0_COAv6O<0zL*kyVu>rn>v9R;Ek(2%B?!UDRPq=K)5yAHRhTUf_5=$yh?0@4C>|wZ=pdF9 z6+WiQNH0_dF6bVR4jciOe}zL+XY4qHolFM_Yv^%lBj}S9VB`@E=PVVOVKE5Li_KBt zZZQU+QDiDqjJafCMV3ma;-a$XNId0Asf=-sBID12A@0{I3Qh}3N*(Sh>t7`XuvQJ$)SHe@USUm z0ir+$Jn)drU~dX0QU{JFQ7nRzp+H|`Ms=YzQo^{T#ugw8SYk-oSQ&&&If@CQ0Cw9X z0<})WWMx!n1!{$e(Q*Y6hm3Ma4e7|GSWpKGt~i1k0O?&6u4Zq!{6#L)On}ulmoy2PCfq@-PW|8G;jnaLz}Cv zLE$_~1MkqRFU}$~c0F~zvSKRddHKo;C?cS$=qywShn0dv^MFeqj4L{#6q!#t?p2{O zYay8|R^^}|9!x;cCG!T2UzPc5zENE&Ck+N4A;+w62O>yRIBQ*)h|**uva*o5;)=w9 z0~%W8JgzeJnL#dW2q>bBXdxMKGO5Z$UJ7Vhbb{PhI)btx3xjUgEZayYjr9&K1Y$EZ z4_X7FBCMvxyA0l-ZA5KqmtqWxqlX>TJXgtuYFYF_rp2jbqGn*AEY2(3l8H$;sqKWD zxSW5U zv1kY{&SL~F!GBAkk*Fol3awBw`VEqa6Tmy4WR$F5Yl1%=n>rb> z8HcucHw*LGd0~VxGQXTLPUhdw6eY$|GGES4{_ViR>FM2E!&oOC7X5Iu-8>@mpJwyl z%uasypT71_>@Vim-G*S2hT!d{8Aj4ylJUAF@0#_tS)Cqun}#du(tbK*Z{+#mWW^bo zW7F7v7|BJu>fe)PyIBm~1;#GkUpM1qeeC2E!D4&1 zH@&~eA8y*kh@cLwlFitR?PKyPzj#`n=Ct3vznJe}=@$L^F~RHG7dby8i*8sT)tUv; zuVXWqjaj_C_Zyn^x*et`>QlSt&d7t*FShUMm{H9W;Z%TcLu!`~?w@~&z_uGQ^zU~z zc7Cgu-;xKn%a891|1}%2j@C>7wzq~pwQ;cgn9M7r2LX~hXto=0s#YC8m4st|bGmAOZdYW{3`^pl-NT8y!^eZUpRV$yCCfW3VBrpf zv)7OI;6HfPweQK|K8*(;-1_Z~#FNkN9SG69yO}7rA$^IULSk)I<@9!~&sjkoO=`Z(jhk>up_DPD^Kf8yIpZmoe$rt-D v+;ZS!92zwBFpcoN?Ecykhf%K{^J;arZyGWK{=+!sSO+%D?BT=whcf#YI%lqo literal 0 HcmV?d00001 diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 123beddfbc041..b30a844f1e6e2 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -2147,6 +2147,7 @@ describe("api", function () { revisionIndex: 0, parentId: null, coversWholeDocument: true, + modificationsAfterSignature: 0, }, { id: "49R:0-60285-74083-357914", @@ -2163,6 +2164,7 @@ describe("api", function () { revisionIndex: 1, parentId: "70R:0-435961-437515-34098", coversWholeDocument: false, + modificationsAfterSignature: 1, }, ]); @@ -2202,6 +2204,81 @@ describe("api", function () { await loadingTask.destroy(); }); + it("gets signature metadata without fetching later revisions", async function () { + const baseData = await DefaultFileReaderFactory.fetch({ + path: TEST_PDFS_PATH + "signed_verified.pdf", + }); + const payload = new Uint8Array(2 * 65536).fill(0x41); + const objectHeader = stringToBytes( + `\n9 0 obj\n<< /Length ${payload.length} >>\nstream\n` + ); + const objectFooter = stringToBytes("\nendstream\nendobj\n"); + const objectOffset = baseData.length + 1; + const xrefOffset = + baseData.length + + objectHeader.length + + payload.length + + objectFooter.length; + const xref = stringToBytes( + "xref\n" + + "9 1\n" + + `${objectOffset.toString().padStart(10, "0")} 00000 n \n` + + "trailer\n" + + "<< /Size 10 /Root 1 0 R /Prev 10007 >>\n" + + "startxref\n" + + `${xrefOffset}\n` + + "%%EOF\n" + ); + const data = new Uint8Array( + baseData.length + + objectHeader.length + + payload.length + + objectFooter.length + + xref.length + ); + let position = 0; + for (const part of [ + baseData, + objectHeader, + payload, + objectFooter, + xref, + ]) { + data.set(part, position); + position += part.length; + } + + const initialData = new Uint8Array(data.subarray(0, 65536)); + const transport = new PDFDataRangeTransport(data.length, initialData); + let fetches = 0; + transport.requestDataRange = (begin, end) => { + fetches++; + waitSome(() => { + transport.onDataRange( + begin, + new Uint8Array(data.subarray(begin, end)) + ); + }); + }; + + const loadingTask = getDocument({ + range: transport, + rangeChunkSize: 65536, + disableAutoFetch: true, + disableStream: true, + }); + const pdfDoc = await loadingTask.promise; + const fetchesAfterLoading = fetches; + const signatures = await pdfDoc.getSignatures(); + + expect(signatures.length).toEqual(1); + expect(signatures[0].coversWholeDocument).toEqual(false); + expect(signatures[0].modificationsAfterSignature).toEqual(1); + expect(fetches).toEqual(fetchesAfterLoading); + + await loadingTask.destroy(); + }); + it("gets non-existent calculationOrder", async function () { const calculationOrder = await pdfDocument.getCalculationOrderIds(); expect(calculationOrder).toEqual(null); diff --git a/test/unit/document_spec.js b/test/unit/document_spec.js index 53e599657fbd5..de0de169dbce4 100644 --- a/test/unit/document_spec.js +++ b/test/unit/document_spec.js @@ -17,6 +17,7 @@ import { 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"; +import { XRef } from "../../src/core/xref.js"; describe("document", function () { describe("Page", function () { @@ -44,6 +45,35 @@ describe("document", function () { }); }); + describe("XRef", function () { + it("compares xref sections with absolute file offsets", function () { + const stream = new StringStream(" ".repeat(200)); + stream.pos = 10; + stream.moveStart(); + + const xref = new XRef(stream, {}); + xref._xrefSectionOffsets.add(100); + + expect(xref.countUpdatesAfter(105)).toEqual(1); + expect(xref.countUpdatesAfter(111)).toEqual(0); + }); + + it("returns an unknown update count for an incomplete xref chain", function () { + const stream = new StringStream( + "xref\n" + + "0 1\n" + + "0000000000 65535 f \n" + + "trailer\n" + + "<< /Size 1 /Prev 999 >>\n" + ); + const xref = new XRef(stream, {}); + xref.setStartXRef(0); + + expect(xref.readXRef()).toBeInstanceOf(Dict); + expect(xref.countUpdatesAfter(0)).toBeNull(); + }); + }); + describe("PDFDocument", function () { // Padded to 1024 bytes so signature ByteRange tests using offsets // like `[0, 100, 200, 800]` stay within `stream.end` (the new @@ -51,7 +81,11 @@ describe("document", function () { // the file length). const stream = new StringStream("Dummy_PDF_data".padEnd(1024, " ")); - function getDocument(acroForm, xref = new XRefMock()) { + function getDocument( + acroForm, + xref = new XRefMock(), + documentStream = stream + ) { const catalog = { acroForm }; const pdfManager = { get docId() { @@ -74,7 +108,7 @@ describe("document", function () { return { isOffscreenCanvasSupported: false }; }, }; - const pdfDocument = new PDFDocument(pdfManager, stream); + const pdfDocument = new PDFDocument(pdfManager, documentStream); pdfDocument.xref = xref; pdfDocument.catalog = catalog; @@ -283,6 +317,54 @@ describe("document", function () { expect(bytes.data.length).toEqual(2); }); + it("reports whole-document coverage when only whitespace trails the signed range", async function () { + const acroForm = new Dict(); + acroForm.set("SigFlags", 3); + + const sigRef = Ref.get(40, 0); + const fieldRef = Ref.get(41, 0); + const sigDict = makeSigDict({ byteRange: [0, 20, 30, 20] }); + const fieldDict = makeSigField({ T: "sig_full", sigRef }); + + const xref = new XRefMock([ + { ref: sigRef, data: sigDict }, + { ref: fieldRef, data: fieldDict }, + ]); + acroForm.set("Fields", [fieldRef]); + + const documentStream = new StringStream( + "A".repeat(50) + " ".repeat(150) + ); + const pdfDocument = getDocument(acroForm, xref, documentStream); + const signatures = await pdfDocument.signatures; + expect(signatures.length).toEqual(1); + expect(signatures[0].coversWholeDocument).toBeTrue(); + }); + + it("clears whole-document coverage when non-whitespace trails the signed range", async function () { + const acroForm = new Dict(); + acroForm.set("SigFlags", 3); + + const sigRef = Ref.get(42, 0); + const fieldRef = Ref.get(43, 0); + const sigDict = makeSigDict({ byteRange: [0, 20, 30, 20] }); + const fieldDict = makeSigField({ T: "sig_partial", sigRef }); + + const xref = new XRefMock([ + { ref: sigRef, data: sigDict }, + { ref: fieldRef, data: fieldDict }, + ]); + acroForm.set("Fields", [fieldRef]); + + const documentStream = new StringStream( + "A".repeat(50) + "MODIFIED" + " ".repeat(142) + ); + const pdfDocument = getDocument(acroForm, xref, documentStream); + const signatures = await pdfDocument.signatures; + expect(signatures.length).toEqual(1); + expect(signatures[0].coversWholeDocument).toBeFalse(); + }); + it("walks Kids recursively to find nested signature fields", async function () { const acroForm = new Dict(); acroForm.set("SigFlags", 3); diff --git a/web/digital_signature_properties_manager.js b/web/digital_signature_properties_manager.js index 1f956c732adc0..95c0f363fdc85 100644 --- a/web/digital_signature_properties_manager.js +++ b/web/digital_signature_properties_manager.js @@ -270,6 +270,7 @@ class SignaturePropertiesManager { message: null, certificate: null, documentModifiedAfterSigning: !sig.coversWholeDocument, + modificationsAfterSignature: sig.modificationsAfterSignature, }); } this.#render(); @@ -647,6 +648,7 @@ class SignaturePropertiesManager { message: ex?.message ?? null, certificate: null, documentModifiedAfterSigning: !signature.coversWholeDocument, + modificationsAfterSignature: signature.modificationsAfterSignature, }; } this.#pendingVerify.delete(signature.id); diff --git a/web/firefoxcom.js b/web/firefoxcom.js index 65f9aff46a27a..7da64a4efc265 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -593,6 +593,7 @@ class SignatureVerifier { message: signature.subFilter, certificate: null, documentModifiedAfterSigning: !signature.coversWholeDocument, + modificationsAfterSignature: signature.modificationsAfterSignature, }; } @@ -610,6 +611,7 @@ class SignatureVerifier { message: ex?.message ?? null, certificate: null, documentModifiedAfterSigning: !signature.coversWholeDocument, + modificationsAfterSignature: signature.modificationsAfterSignature, }; } if (!response || response.error) { @@ -619,6 +621,7 @@ class SignatureVerifier { message: null, certificate: null, documentModifiedAfterSigning: !signature.coversWholeDocument, + modificationsAfterSignature: signature.modificationsAfterSignature, }; } @@ -632,6 +635,7 @@ class SignatureVerifier { message: null, certificate: null, documentModifiedAfterSigning: !signature.coversWholeDocument, + modificationsAfterSignature: signature.modificationsAfterSignature, }; } const { status, errorCode } = mapVerificationStatus( @@ -644,6 +648,7 @@ class SignatureVerifier { message: entry.message ?? null, certificate: entry.certificate ?? null, documentModifiedAfterSigning: !signature.coversWholeDocument, + modificationsAfterSignature: signature.modificationsAfterSignature, }; } diff --git a/web/genericcom.js b/web/genericcom.js index 33e573080d2b6..6209cf2285ff9 100644 --- a/web/genericcom.js +++ b/web/genericcom.js @@ -184,6 +184,7 @@ if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) { message: signature.subFilter, certificate: null, documentModifiedAfterSigning: !signature.coversWholeDocument, + modificationsAfterSignature: signature.modificationsAfterSignature, }; } @@ -193,6 +194,7 @@ if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) { message: null, certificate: null, documentModifiedAfterSigning: !signature.coversWholeDocument, + modificationsAfterSignature: signature.modificationsAfterSignature, }; }