Skip to content

Work the 2026-09-21 security audit: invisible characters, homoglyph folds, web sinks, --png scratch name - #4

Merged
bmmmm merged 8 commits into
mainfrom
worktree-security-remediate-2026-09-21
Sep 21, 2026
Merged

bmmmm merged 8 commits into
mainfrom
worktree-security-remediate-2026-09-21

Conversation

@bmmmm

@bmmmm bmmmm commented Sep 21, 2026

Copy link
Copy Markdown
Owner

Works the seven findings of the 2026-09-21 security audit (snapshot sha256 c9c41bc53b58…; every commit carries an Audit: c9c41bc53b58#Fxx trailer). All seven were low; the two the audit had drafted as medium (F01, F02) share one root cause and one fix.

Claims

  • F01/F02 — every payload field refuses invisible characters, not only CR/LF. internal/epc/epc.go (invisibleRune) rejects Unicode Cc (ESC, NUL, TAB, DEL, C1) and Cf (bidi overrides such as U+202E, zero-width space/joiner, BOM) with exit 2 and an error naming field and codepoint, e.g. beneficiary name contains a control character U+001B. Field labels in these errors now match the length errors (beneficiary name, remittance text, …) instead of name/text. No-break spaces and combining marks stay legal. Names containing emoji joined by U+200D are now refused.
  • F01 — --details can no longer drive the terminal. printDetails (main.go, graphic) renders any non-graphic rune as \uXXXX; before, --text $'…\e[2A\e[2K iban: DE00…' rewrote the printed iban: line while the QR carried the real IBAN.
  • F03 — --iban, --bic, --purpose and an RF creditor reference in --ref name a non-ASCII character instead of case-folding it (asciiUpper). ſNPAFRPP (U+017F) was encoded as the BIC SNPAFRPP, GB82WEſT… validated as GB82WEST…, RF35ſ12345 became the checksum-valid RF35S12345. A reference that is not an RF claim still passes through verbatim, non-ASCII included.
  • F04 — Web: the QR is inserted as a parsed SVG node (DOMParser + replaceChildren(importNode(…))) instead of innerHTML; a document that does not parse to an <svg> root shows an error instead of a stale code. scripts/web-smoke.mjs fails on any HTML string sink in web/.
  • F05 — Web: the download filename comes from the encoded payload, not the live IBAN field (150 ms debounce race). Gated in web-smoke.mjs.
  • F06 — Web/README: the privacy note says what the browser does with a share link ("not sent in requests … stored in your browser history like any other URL") instead of "never send to any server".
  • F07 — --png scratch file gets a crypto/rand suffix instead of the pid; a file planted under the old predictable name no longer blocks the write. Mode semantics unchanged (new: os.Create default; replaced: kept), symlink-follow unchanged as the audit accepted.
  • Docs: CONTRIBUTING "Payload rules" carries the new invisible-character rule; CHANGELOG [Unreleased] lists every behaviour change with the input that behaves differently.

Verification

$ go test ./...
ok  	github.com/bmmmm/epcii	0.242s
ok  	github.com/bmmmm/epcii/internal/epc	0.238s
ok  	github.com/bmmmm/epcii/internal/qr	(cached)
ok  	github.com/bmmmm/epcii/internal/render	(cached)
ok  	github.com/bmmmm/epcii/internal/webapi	0.229s
$ go vet ./... && gofmt -l .        # clean, nothing listed

$ node scripts/web-smoke.mjs ./epcii
ok   svg (5084 bytes identical)
ok   png (1921 bytes identical)
ok   invalid IBAN → "a DE IBAN has 22 characters, got 4"
ok   version v0.2.0-17-gc041043
ok   web/index.html names no storage, address-bar write, external resource or HTML string sink
ok   web/app.js names no storage, address-bar write, external resource or HTML string sink
ok   web/style.css names no storage, address-bar write, external resource or HTML string sink
ok   web/app.js fileStem() derives the name from the encoded result

# F01 at 5cb5275 (before), stderr through od -c:
  n a m e :   A l i c e 033 [ 3 1 m E V I L \n         exit=0
# F01 at HEAD, same input:
  epcii: beneficiary name contains a control character U+001B\n   exit=2, stdout 0 bytes
# F03 before: bic: SNPAFRPP  exit=0
# F03 after:  epcii: BIC contains a non-ASCII character U+017F ('ſ'); only A-Z and 0-9 are valid   exit=2

Browser (Chrome against the built web/dist): exactly one <svg> child in #qr, namespace http://www.w3.org/2000/svg, serialisation equal to last.svg; fileStem() stayed at the encoded IBAN while the field already held another; footer EN/DE show the history sentence; U+202E / ESC / ſ inputs show the gate's message on the WASM path and hide the QR; no console errors.

Tests

Each was seen red at 5cb5275 before its fix (a fresh closer agent repeated that from a git archive export of 5cb5275 with the new test files copied over; a reviewer's revert-mutation turned all seven Go gates red at once):

  • TestPayloadRejectsInvisibleCharacters (internal/epc) — 13 cases accepted before; plus a good-case block (NBSP, combining marks, tab inside the IBAN).
  • FuzzPayload — seeds 1–3 carried U+001B / U+202E before; invariant: ≤ 12 lines, no Cc/Cf beyond LF.
  • TestRunDetailsRejectsEscape, TestPrintDetailsEscapesNonGraphic (main_test.go) — exit 0 and raw 033 on stderr before.
  • TestValidateIBAN (GB82WEſT…), TestPayloadRejectsNonASCIIBIC (ſ, ı, U+212A, U+FF22 in BIC; ſ in purpose; ſ in an RF reference; non-RF reference with umlauts must stay verbatim) — accepted before.
  • web-smoke.mjs gates "HTML string sink" and "fileStem() derives the name from the encoded result" — both FAIL before.
  • TestRunPNGScratchNameUnpredictablefile exists on the decoy before.
  • F06 is wording; the check was a one-off grep (never send|nie an einen Server senden: 4 hits → 0), not a permanent gate.

Public contracts

Confirm none of these changed, or list the break under Claims:

  • stdout carries only the SVG — previews, details, and errors go to stderr (TestRunDetailsRejectsEscape asserts empty stdout on refusal)
  • exit codes: 0 success, 2 invalid input/usage, 1 I/O failure
  • CLI flag names and semantics are unchanged
  • payload format: EPC069-12 v002, LF separators, UTF-8 (charset 1), two-decimal amounts, trailing empty fields trimmed — unchanged for every accepted input; the accepted input set is narrower (invisible characters, non-ASCII in ASCII identifiers) — listed under Claims and in CHANGELOG
  • zero runtime dependencies (go.mod/go.sum untouched; crypto/rand, encoding/hex, io/fs, unicode are stdlib)

--details output format changed for non-graphic characters only (\uXXXX escapes instead of raw bytes).

Out of scope

Leads for the next audit, recorded in ~/ops/audits/remediation/2026-09-21-remediation-epcii.md:

  • render.SVG's "geometry only" invariant is still enforced by nothing but a comment (TestSVGStructure asserts presence, not absence of <script>/on*=); the audited sink and its gate are closed one layer above.
  • U+2028 / U+2029 (Zl/Zp) pass the new gate; --details escapes them, the web <pre> breaks the line.
  • The Cf refusal has no "retype the text" hint (LRM/RLM from Outlook, soft hyphen, ZWNJ/ZWJ in Persian/Devanagari/emoji).
  • The two new smoke gates are grep gates: readForm().iban inside fileStem() and setHTMLUnsafe( slip past.
  • app.js: the DOMParser block could also check namespaceURI and sit inside the fail-closed try/catch.
  • TestRunPNGScratchNameUnpredictable proves "not the pid", not "random"; the retry bound is untested.
  • internal/qr untouched; no NFC normalisation (zero-dependency contract, CONTRIBUTING).

AI assistance

  • I reviewed every line of this PR and can explain it
  • I ran the verification commands myself

Authored in an agent session (Claude Code) from the audit snapshot; closure of each finding was judged by a second agent that never saw the fixer's reasoning, and the diff was reviewed by a third. The human maintainer has not yet reviewed — hence draft.

… in --details

Only CR and LF were filtered; ESC, NUL, TAB, the C1 range and every Unicode
format character (bidi overrides, zero-width space and joiner, BOM) reached
the payload and were printed raw by --details. An ESC sequence in --text
could rewrite the iban: line on the terminal while the QR carried the real
IBAN; a U+202E in --name makes the beneficiary read differently from how it
is stored. The field gate now rejects Cc and Cf runes naming the field and
codepoint, the field labels match the length errors, and printDetails
renders any non-graphic rune as its \u escape so the verification view can
never drive the terminal it is printed on.

Checks seen red at 5cb5275: TestPayloadRejectsInvisibleCharacters (13 cases
accepted), FuzzPayload seeds 1-3, TestRunDetailsRejectsEscape (exit 0, raw
ESC on stderr), TestPrintDetailsEscapesNonGraphic.

Snapshot: ~/ops/audits/2026-09-21-security-epcii.md

Audit: c9c41bc53b58#F01
Audit: c9c41bc53b58#F02
…olding it

strings.ToUpper turns U+017F (long s) into S and U+0131 (dotless i) into I,
so a pasted homoglyph was validated in a form the user never typed: the BIC
carries no checksum, so ſNPAFRPP became SNPAFRPP without a word, and
GB82WEſT… validated as GB82WEST…. asciiUpper now refuses anything outside
ASCII before the fold, naming field and codepoint; ASCII lower case still
folds. The purpose code sits three lines from the BIC in the same function
and used the same fold, so it takes the same guard.

Checks seen red at d6ae48a: TestValidateIBAN (GB82WEſT… accepted),
TestPayloadRejectsNonASCIIBIC (long s and dotless i accepted, Kelvin sign
and fullwidth B refused only by byte length).

Snapshot: ~/ops/audits/2026-09-21-security-epcii.md

Audit: c9c41bc53b58#F03
…e smoke test

The page assigned render.SVG's output as an HTML string, safe only because
that renderer emits geometry — an invariant asserted by a comment in another
module, which nothing enforced. The SVG is now parsed as image/svg+xml and
inserted as a node; a document that does not parse to an <svg> root shows
an error instead of a stale code. web-smoke.mjs fails on any HTML string
sink (innerHTML, outerHTML=, insertAdjacentHTML, document.write) in the
page sources, so the invariant is mechanical from here on.

Check seen red at 902e1bf: web-smoke.mjs 'web/app.js violates the
zero-storage contract: HTML string sink'. SVG and PNG stay byte-identical
to the CLI (5084 / 1921 bytes).

Snapshot: ~/ops/audits/2026-09-21-security-epcii.md

Audit: c9c41bc53b58#F04
.<name>.<pid>.tmp was guessable: a file planted under that name made every
--png into the directory fail with 'file exists', and in a non-sticky
shared directory it marked the window between close and rename. The suffix
is now 16 hex characters from crypto/rand; O_EXCL still refuses to follow a
planted symlink, a collision gets a fresh suffix. Mode semantics are
untouched (0666 minus umask for new files, kept mode for replaced ones),
which is why os.CreateTemp — which forces 0600 — was not used.

Check seen red at 6c1b9f3: TestRunPNGScratchNameUnpredictable (exit 1,
'file exists' on the decoy).

Snapshot: ~/ops/audits/2026-09-21-security-epcii.md

Audit: c9c41bc53b58#F07
fileStem() read the IBAN input live while the SVG/PNG blobs came from the
cached last result; rendering is debounced by 150 ms, so a click inside
that window saved epc-<new IBAN>.svg carrying the previous IBAN's code.
The stem now comes from payload line 7 of the result that is being
downloaded, and web-smoke.mjs fails if fileStem() ever reads the DOM again.

Check seen red at 93764f8: web-smoke.mjs 'fileStem() reads the form
instead of the encoded result'.

Snapshot: ~/ops/audits/2026-09-21-security-epcii.md

Audit: c9c41bc53b58#F05
The footer promised that browsers 'never send' the #fragment 'to any
server'. True of requests, but the URL including the fragment sits in the
browser history, travels with history sync and reaches a search provider
through address-bar suggestions. The footer (EN/DE) and README 'Web' now
say 'not sent in requests' and name the history.

Check seen red at 93764f8: grep 'never send|nie an einen Server senden'
over web/ and README.md — 4 hits; 0 after.

Snapshot: ~/ops/audits/2026-09-21-security-epcii.md

Audit: c9c41bc53b58#F06
The fourth ASCII-by-definition identifier kept the generic fold: an RF
claim was upper-cased before its mod-97 check, so RF35ſ12345 (U+017F)
became the checksum-valid RF35S12345 and went into the QR. The claim is now
routed through asciiUpper like BIC, purpose and IBAN. A reference that is
not an RF claim belongs to an issuer's scheme and still passes through
verbatim, non-ASCII included — the test pins that path as well.

Check seen red at c041043: TestPayloadRejectsNonASCIIBIC 'long s in RF
reference: accepted, payload ...RF35S12345'. Found by the phase-8 closer
(PARTIAL) and the reviewer (P1) independently.

Also moves the pre-existing DefaultPNGScale bullet back under Changed; it
had slipped under the new Fixed heading.

Snapshot: ~/ops/audits/2026-09-21-security-epcii.md

Audit: c9c41bc53b58#F03
README 'Web', the CONTRIBUTING module map and the smoke's own success line
still described the gate as byte identity plus storage/address-bar/external
resource greps; it also refuses HTML string sinks and pins fileStem() to the
encoded result since 93764f8 and 1f10904.
@bmmmm
bmmmm marked this pull request as ready for review September 21, 2026 11:32
@bmmmm
bmmmm merged commit 27bee68 into main Sep 21, 2026
4 checks passed
@bmmmm
bmmmm deleted the worktree-security-remediate-2026-09-21 branch September 21, 2026 11:32

This branch was successfully deployed

1 active deployment
github-pages 27bee684 Deployed Sep 21, 2026 by bmmmm via deploy #3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant