ADR 0026. Both ADR 0024 and ADR 0025 said this code needed a security
review before shipping. This is that review, done adversarially: for each
way a signature could be defeated, a test that attempts it. It found a
critical vulnerability in the code as shipped last turn.
FINDING 1, critical, exploitable with no special access.
Verification recovered the certificate and the signature by *scanning* the
blob for DER-shaped bytes rather than decoding it. The signature was checked
against certificates[0]; trust was checked against ANY certificate present.
Two questions, two different certificates. So:
the attacker signs a forgery with their own key
the attacker appends the victim's trusted certificate to the blob
signature_valid = true (their signature over their own content is real)
chain_trusted = true (the victim's certificate is present)
is_valid() = true
Demonstrated before the fix, with the message "I hereby transfer everything
to the attacker" verifying as valid.
Fixed by decoding the ContentInfo/SignedData structure and finding the
certificate the SignerInfo actually names, by issuer AND serial, then
evaluating both the signature and the trust path against that one
certificate. Trailing data now fails the decode instead of being ignored.
The scanning functions are deleted, not left unused: dead code that once
returned the wrong answer is an invitation to call it again.
FINDING 2, moderate. signer_certificate() returned chain[0] unconditionally,
so a chain whose first entry was not the signing key's certificate made the
SignerInfo name the wrong one. Not a forgery route — the signature fails —
but a UI showing "signed by <somebody trustworthy>" beside a failed check is
its own kind of dangerous. Now it finds the entry whose public key matches
the key doing the signing.
FINDING 3, informational. digest_matches was hardcoded true under a comment
claiming it was computed. Not exploitable, because is_valid() also requires
signature_valid and the signature covers the bytes — but a field asserting
an unperformed check is ADR 0017's pattern exactly.
The four items ADR 0025 left unticked are closed:
PKIX chain building, with each link's issuer signature verified. A name
match alone is not a chain; anyone can put any name in a certificate.
Pinning still short-circuits first.
Stapled revocation from /DSS, offline only. Unknown is the default and a
first-class answer: treating "no information" as "not revoked" is a claim
a verifier cannot support.
Signature appearances, with the claimed time labelled "Time claimed"
because a self-declared /M carries no authority.
One-call sign_document. Three things were wrong first: the /ByteRange
placeholder was too narrow for real offsets so patching them moved every
later byte; /Contents must be a hex string because a literal full of NULs
needs escaping and changes length; and a signature dictionary nothing
points at is invisible — the first version wrote one and the reader
reported zero signatures over a correctly signed document.
Four mutations, all killed — two only after strengthening the tests. My
first smuggling test put the attacker's certificate first, where
certificates[0] finds it anyway, so it passed with or without the
issuer/serial match. Putting the TRUSTED certificate first is what
distinguishes them, and writing that test is what exposed Finding 2.
qpdf --check accepts the signed documents. pdf: 1276 passed. Coverage 87.98%.
Left unticked, deliberately: an independent review by someone who did not
write the code. This is a self-review; it found two real vulnerabilities,
which is evidence the method works and not evidence that nothing remains.
Also untested against Acrobat, which is stricter than the spec, and
sign_document replaces rather than merges an existing AcroForm.
