From f2245875cc694f397eaffc1b8aa5e96b019cbd77 Mon Sep 17 00:00:00 2001 From: Patrick Lensing Date: Wed, 5 Aug 2026 00:09:16 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20extractSignature:=20read=20DER?= =?UTF-8?q?=20length=20instead=20of=20stripping=20trailing=2000=20pairs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trailing-byte strip (replace(/(?:00|>)+$/)) cannot distinguish sign()'s zero-padding from a genuine signature whose last byte(s) happen to be 0x00 (~1/256 chance per signature), so it intermittently truncated valid DER and downstream parsers failed with "Too few bytes to read ASN.1 value." Read the total element length from the DER SEQUENCE header instead; fall back to the historical trailing-zero strip when the blob is not parseable DER. Fixes #317. Co-Authored-By: Claude Fable 5 --- packages/utils/src/extractSignature.js | 43 ++++++++++++++++++- packages/utils/src/extractSignature.test.js | 46 +++++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/packages/utils/src/extractSignature.js b/packages/utils/src/extractSignature.js index 6f47a09b..0dbfb11d 100644 --- a/packages/utils/src/extractSignature.js +++ b/packages/utils/src/extractSignature.js @@ -11,6 +11,33 @@ const getSubstringIndex = (str, substring, n) => { return index; }; + +/** + * Total encoded length (header + content) of the DER element starting at buf[0]. + * Returns -1 if the buffer does not start with a parseable SEQUENCE header. + * @param {Buffer} buf + * @returns {number} + */ +const derTotalLength = (buf) => { + if (buf.length < 2 || buf[0] !== 0x30) { + return -1; + } + const lengthByte = buf[1]; + if ((lengthByte & 0x80) === 0) { + // Short form: the byte is the content length itself. + return 2 + lengthByte; + } + const numLengthOctets = lengthByte & 0x7f; + if (numLengthOctets === 0 || buf.length < 2 + numLengthOctets) { + return -1; + } + let contentLength = 0; + for (let i = 0; i < numLengthOctets; i += 1) { + contentLength = contentLength * 256 + buf[2 + i]; + } + return 2 + numLengthOctets + contentLength; +}; + /** * Basic implementation of signature extraction. * @@ -62,9 +89,21 @@ export const extractSignature = (pdf, signatureCount = 1) => { const signatureHex = pdf.slice(ByteRange[0] + ByteRange[1] + 1, ByteRange[2]) .toString('binary') - .replace(/(?:00|>)+$/, ''); + .replace(/(?:>|\s)+$/, ''); + + // sign() right-pads the signature with zero bytes up to the placeholder + // length. DER is self-describing, so read the real element length from the + // SEQUENCE header instead of stripping trailing "00" pairs — a genuine + // signature has a ~1/256 chance of ending in 0x00 itself, and the old + // regex-based strip would eat those real bytes and corrupt the DER (#317). + const padded = Buffer.from(signatureHex, 'hex'); + const totalLength = derTotalLength(padded); + const signatureBuffer = totalLength > 0 && totalLength <= padded.length + ? padded.slice(0, totalLength) + // Not parseable as DER — keep the historical trailing-zero strip. + : Buffer.from(signatureHex.replace(/(?:00)+$/, ''), 'hex'); - const signature = Buffer.from(signatureHex, 'hex').toString('binary'); + const signature = signatureBuffer.toString('binary'); return { ByteRange: matches.slice(1, 5).map(Number), diff --git a/packages/utils/src/extractSignature.test.js b/packages/utils/src/extractSignature.test.js index 97a8d661..87190e9e 100644 --- a/packages/utils/src/extractSignature.test.js +++ b/packages/utils/src/extractSignature.test.js @@ -43,4 +43,50 @@ describe(extractSignature, () => { const extracted = extractSignature(signedPdf); expect(extracted).toMatchSnapshot(); }); + + /** + * Builds a minimal fake signed PDF: a self-consistent /ByteRange header, + * a /Contents-style placeholder holding derBuffer + zero padding, + * and a trailer. + */ + const makeFakeSignedPdf = (derBuffer, paddingBytes) => { + const hex = derBuffer.toString('hex') + '00'.repeat(paddingBytes); + const placeholder = `<${hex}>`; + const trailer = 'trailer'; + const pad = (n) => String(n).padStart(10, '0'); + // Fixed-width numbers keep the header length independent of the values. + const prefix = '%PDF-1.7\n'; + const headerFor = (b1, b2, b3) => `/ByteRange [0 ${pad(b1)} ${pad(b2)} ${pad(b3)}]`; + const b1 = prefix.length + headerFor(0, 0, 0).length; // offset of '<' + const b2 = b1 + placeholder.length; // just past '>' + const header = headerFor(b1, b2, trailer.length); + return Buffer.from(prefix + header + placeholder + trailer, 'binary'); + }; + + it('keeps a genuine trailing 0x00 byte of the signature (#317)', () => { + // DER SEQUENCE whose real content ends in 0x00: + // 30 05 (SEQUENCE, len 5) 04 03 aa bb 00 (OCTET STRING, len 3) + const der = Buffer.from('30050403aabb00', 'hex'); + const extracted = extractSignature(makeFakeSignedPdf(der, 20)); + expect(Buffer.from(extracted.signature, 'binary')).toEqual(der); + }); + + it('trims placeholder padding from a long-form-length DER ending in 0x00', () => { + // SEQUENCE with long-form length: 30 82 01 06, then 262 content bytes + // (a 258-byte OCTET STRING incl. its 4-byte header) ending in 0x00. + const content = Buffer.concat([ + Buffer.from('04820102', 'hex'), + Buffer.alloc(258, 0xab), + ]); + content[content.length - 1] = 0x00; + const der = Buffer.concat([Buffer.from('30820106', 'hex'), content]); + const extracted = extractSignature(makeFakeSignedPdf(der, 40)); + expect(Buffer.from(extracted.signature, 'binary')).toEqual(der); + }); + + it('extracts an unpadded signature unchanged', () => { + const der = Buffer.from('30050403aabbcc', 'hex'); + const extracted = extractSignature(makeFakeSignedPdf(der, 0)); + expect(Buffer.from(extracted.signature, 'binary')).toEqual(der); + }); });