Skip to content

extractSignature(): trailing-zero regex strip can truncate a genuine signature byte (~1/256 chance), corrupting DER #317

Description

@gloomydumber

Summary

extractSignature()'s trailing-byte stripping (.replace(/(?:00|>)+$/, '') in extractSignature.js) can silently truncate a genuine signature byte, not just sign()'s zero-padding — corrupting the extracted DER and making an otherwise-valid signature fail to parse.

I understand from #206/#208 that extractSignature is considered internal/deprecated and slated for removal in a future major version. Filing this anyway since it's still exported from the current published @signpdf/utils (3.3.0) and I couldn't find an existing report of this specific failure mode — hopefully useful to anyone still relying on it before the removal lands.

Root cause

sign() right-pads the hex-encoded signature with \x00 bytes up to the placeholder length, then extractSignature() tries to strip that padding back off with:

const signatureHex = pdf.slice(ByteRange[0] + ByteRange[1] + 1, ByteRange[2])
  .toString('binary')
  .replace(/(?:00|>)+$/, '');

This regex can't distinguish "signpdf's padding" from "the real signature's own last byte(s) happening to be 0x00". A PKCS#7/CMS SignedData DER structure's final bytes are the raw RSA signature (an OCTET STRING of effectively-random bytes for a given key), so there's a ~1/256 chance per trailing byte that the genuine content ends in 0x00. When it does, the regex eats real DER content along with the padding, and any downstream parser (e.g. forge.asn1.fromDer) throws "Too few bytes to read ASN.1 value." because the buffer now ends before the length the DER header declares.

Because this depends only on the signature's own bytes (not on document content or size), it's intermittent and looks like a flaky ~1-in-a-few-hundred-signatures failure, which makes it easy to write off as environmental noise rather than a real bug.

Repro

Using @signpdf/signer-p12's P12Signer directly (any RSA key/cert will do — this is independent of PDF content):

const { P12Signer } = require('@signpdf/signer-p12');
const p12 = /* your p12 buffer */;

let raw;
for (let i = 0; ; i++) {
  const signer = new P12Signer(p12, { passphrase: '...' }); // fresh instance: forge's ByteBuffer is consumed on parse
  raw = Buffer.from(await signer.sign(Buffer.from(`probe ${i} ${Date.now()}`)), 'binary');
  if (raw[raw.length - 1] === 0x00) break; // ~1/256 chance per attempt, usually found within a few hundred tries
}

// Simulate sign()'s padding + extractSignature()'s slice/strip:
const placeholderLength = 8192; // hex chars
let hex = raw.toString('hex');
hex += Buffer.from('\x00'.repeat(placeholderLength / 2 - raw.length)).toString('hex');
const hexWithBracket = hex + '>'; // what extractSignature() sees after its off-by-one slice

const stripped = Buffer.from(hexWithBracket.replace(/(?:00|>)+$/, ''), 'hex');
console.log(stripped.length, 'vs original', raw.length); // stripped is 1 byte short

const forge = require('node-forge');
forge.asn1.fromDer(forge.util.createBuffer(stripped.toString('binary')));
// throws: Too few bytes to read ASN.1 value.

Suggested direction

Rather than regex-guessing where padding starts, the placeholder length is already known (it's the signatureLength used to build the placeholder), and DER is self-describing — the total element length can be read directly from the SEQUENCE header (tag + length bytes) instead of inferred from trailing zero bytes:

function derElementLength(buf) {
  const lengthByte = buf[1];
  if ((lengthByte & 0x80) === 0) return 2 + lengthByte; // short form
  const n = lengthByte & 0x7f;
  let length = 0;
  for (let i = 0; i < n; i++) length = (length << 8) | buf[2 + i];
  return 2 + n + length; // long form
}
// slice(0, derElementLength(fullHexDecodedPlaceholder)) instead of the regex strip

This correctly separates real content from padding regardless of what byte values either one happens to contain. Happy to open a PR with this if useful, despite the deprecation plans — otherwise this report is mostly so it's documented for anyone hitting the same intermittent failure in the meantime.

Environment

  • @signpdf/utils / @signpdf/signpdf / @signpdf/signer-p12 / @signpdf/placeholder-pdf-lib: 3.3.0
  • node-forge: 1.4.0
  • Node.js: v22.21.1

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions