diff --git a/CHANGELOG.md b/CHANGELOG.md index a4079c2..a0b9a52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,9 +26,49 @@ what to check. ### Changed +- Every field refuses invisible characters, not only line breaks: a control + character (ESC, NUL, TAB, DEL, the C1 range) or a format character (bidi + overrides such as U+202E, zero-width space and joiner, BOM) is rejected + with exit 2 and an error naming the field and codepoint, e.g. + `beneficiary name contains a control character U+001B`. Before, such a + value was encoded as given, and `--details` printed it raw — an ESC + sequence in `--text` could rewrite the `iban:` line on the terminal while + the QR carried the real IBAN. Names with emoji joined by U+200D are + affected; a no-break space is not. +- `--details` shows any character without a visible form as its `\uXXXX` + escape instead of the raw byte, so the verification view can never drive + the terminal it is printed on. +- `--iban`, `--bic`, `--purpose` and an RF creditor reference in `--ref` + name a non-ASCII character instead of case-folding it: `ſNPAFRPP` + (U+017F, long s) was silently turned into the BIC `SNPAFRPP`, + `GB82WEſT…` into a valid `GB82WEST…`, and `RF35ſ12345` into the + checksum-valid `RF35S12345`. All are now refused with `contains a + non-ASCII character U+017F ('ſ')`. Plain ASCII lower case still folds as + before, and a reference that is not an RF claim keeps passing through + verbatim, non-ASCII included. +- Web: the QR is inserted as a parsed SVG node instead of an HTML string, + and `web-smoke.mjs` now fails on any HTML string sink in the page sources. + Nothing visible changes; an SVG the page cannot parse shows an error + instead of a stale code. +- `--png` names its scratch file with a random suffix instead of the + process id. A file planted under the old predictable name + (`.out.png..tmp`) made the write fail with "file exists"; now it is + left alone and the PNG is written regardless. Permissions are unchanged: + new files get what `os.Create` would, replaced files keep their mode. - The PNG scale (8 px per module) is one constant, `render.DefaultPNGScale`, shared by `--png` and the web download; behaviour is unchanged. +### Fixed + +- Web: the download filename is taken from the encoded payload, not from the + IBAN field. Rendering is debounced by 150 ms, so a click inside that window + could save `epc-.svg` containing the previous IBAN's code. + `web-smoke.mjs` gates it. +- Web: the privacy note no longer says browsers "never send" the #fragment + "to any server". They do not send it in requests, but the link lands in + the browser history like any URL and travels with history sync and + address-bar suggestions. The footer (EN/DE) and README "Web" say so. + ## [0.2.0] - 2026-09-04 ### Changed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f26a8b5..6dd1070 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,7 +16,7 @@ asks for and why. | `cmd/epcii-wasm` | `js && wasm` entry point exposing `internal/webapi` as `globalThis.epcii` | | `web/` | Static page (HTML/JS/CSS, no framework, no external resources); `web/dist/` is the gitignored build output | | `scripts/build-web.sh` | Assembles `web/dist/` (page files, Go's `wasm_exec.js`, the wasm build) | -| `scripts/web-smoke.mjs` | Node gate: the wasm build's SVG/PNG must equal the CLI's byte for byte | +| `scripts/web-smoke.mjs` | Node gate: the wasm build's SVG/PNG must equal the CLI's byte for byte; also greps `web/` for storage APIs, address-bar writes, external resources and HTML string sinks, and pins that `fileStem()` reads the encoded result, not the form | | `scripts/gen_segno_fixtures.py` | One-shot generator for the segno golden fixtures in `internal/epc/testdata/` | | `scripts/qrfixtures/` | Separate Go module: regenerates the upstream matrix fingerprints in `internal/qr/testdata/` from piglig/go-qr | @@ -121,7 +121,14 @@ allows CRLF too). IBANs outside the SEPA country table are rejected, not merely mod-97 checked: the payload initiates a SEPA credit transfer, which cannot reach them. Text fields are not NFC-normalized (that would pull `golang.org/x/text` into the binary); decomposed input counts every -combining mark as a character, and the length error says so. +combining mark as a character, and the length error says so. No field may +carry an invisible character: control characters (Unicode `Cc`, which +includes ESC, NUL and TAB, not only CR/LF) and format characters (`Cf`: +bidi overrides, zero-width space and joiner, BOM) are rejected with the +field and codepoint named. An ESC would drive the terminal that displays +the `--details` view; a bidi override would make a beneficiary name read +differently from how it is stored. Visible whitespace such as the no-break +space stays legal. ## Issues and PRs diff --git a/README.md b/README.md index b067405..9ad8f4e 100644 --- a/README.md +++ b/README.md @@ -88,12 +88,15 @@ What the page does not do: no further request. (GitHub keeps ordinary access logs for the page load itself, like any web host.) - **Share links stay in the fragment.** "Copy link" and "Share" build a URL - of the form `…/epcii/#v=1&name=…&iban=…&amount=…`; browsers never send the - `#fragment` to a server, so payment data does not reach GitHub's logs or a - `Referer`. The address bar is never written automatically — a link exists - only when you ask for one, and opening one fills the form and renders. - Whoever opens such a link has it in their own browser history, as with any - URL; the page cannot prevent that. + of the form `…/epcii/#v=1&name=…&iban=…&amount=…`; browsers do not send + the `#fragment` in requests, so payment data does not reach GitHub's logs + or a `Referer`. The address bar is never written automatically — a link + exists only when you ask for one, and opening one fills the form and + renders. The link itself is an ordinary URL, though: whoever opens it has + it in their browser history, and a browser that syncs history or feeds the + address bar to a search provider treats it like any other address. The + page cannot prevent that; share such a link as you would share the + payment data itself. - **Content Security Policy.** Pages cannot send HTTP headers, so the policy is a `` tag: `default-src 'none'`, scripts and styles only from the page's own origin, no inline script. Directives that a meta CSP cannot @@ -151,7 +154,9 @@ upstream piglig/go-qr encoder (`go run -C scripts/qrfixtures .`, a separate module so upstream never enters `go.mod`). The web build adds a fifth: `scripts/web-smoke.mjs` runs the wasm through Go's `wasm_exec.js` and compares its SVG and PNG with the CLI byte for byte, then greps the page -sources for storage APIs, address-bar writes and external resources. +sources for storage APIs, address-bar writes, external resources and HTML +string sinks, and pins that the download name comes from the encoded +payload rather than the form. ## Contributing diff --git a/internal/epc/epc.go b/internal/epc/epc.go index c0a1063..ee83934 100644 --- a/internal/epc/epc.go +++ b/internal/epc/epc.go @@ -13,7 +13,8 @@ import ( const MaxPayloadBytes = 331 // Payment holds the beneficiary and transfer data for one EPC QR payload. -// All strings are UTF-8; none may contain line breaks. +// All strings are UTF-8; none may contain a line break, a control character +// or an invisible format character (see Payload). type Payment struct { Name string // beneficiary name, required, <=70 chars IBAN string // beneficiary IBAN, required, mod-97 validated @@ -40,7 +41,10 @@ func (p Payment) Payload() (string, error) { return "", err } - bic := strings.ToUpper(strings.ReplaceAll(p.BIC, " ", "")) + bic, err := asciiUpper("BIC", strings.ReplaceAll(p.BIC, " ", "")) + if err != nil { + return "", err + } if bic != "" { if err := validateBIC(bic); err != nil { return "", err @@ -57,7 +61,10 @@ func (p Payment) Payload() (string, error) { } // EPC069-12 section 2.2: purpose (AT-T007) is 1..4 alphanumeric. - purpose := strings.ToUpper(strings.TrimSpace(p.Purpose)) + purpose, err := asciiUpper("purpose code", strings.TrimSpace(p.Purpose)) + if err != nil { + return "", err + } if len(purpose) > 4 { return "", fmt.Errorf("purpose code must be at most 4 characters, got %q", purpose) } @@ -76,7 +83,13 @@ func (p Payment) Payload() (string, error) { // IBAN. Every other structured reference belongs to an issuer's own // scheme, where case and inner spacing may carry meaning, so it passes // through untouched. - if norm := strings.ToUpper(strings.ReplaceAll(ref, " ", "")); isISO11649Claim(norm) { + if stripped := strings.ReplaceAll(ref, " ", ""); isISO11649Claim(strings.ToUpper(stripped)) { + // An RF claim is ASCII by definition: a homoglyph is named before the + // fold instead of becoming a different, checksum-valid reference. + norm, err := asciiUpper("structured reference", stripped) + if err != nil { + return "", err + } ref = norm if err := validateCreditorReference(ref); err != nil { return "", err @@ -100,14 +113,14 @@ func (p Payment) Payload() (string, error) { {"version", "002"}, {"character set", "1"}, // 1 = UTF-8 {"identification", "SCT"}, - {"bic", bic}, // AT-C002, optional in version 002 (EEA) - {"name", name}, // AT-E001 beneficiary name - {"iban", iban}, // AT-C001 beneficiary IBAN - {"amount", amount}, // AT-T002, optional - {"purpose", purpose}, // AT-T007, optional - {"ref", ref}, // AT-T009 structured remittance (exclusive with text) - {"text", p.Text}, // AT-T009 unstructured remittance - {"info", p.Info}, // beneficiary-to-originator information + {"BIC", bic}, // AT-C002, optional in version 002 (EEA) + {"beneficiary name", name}, // AT-E001 + {"IBAN", iban}, // AT-C001 + {"amount", amount}, // AT-T002, optional + {"purpose code", purpose}, // AT-T007, optional + {"structured reference", ref}, // AT-T009 structured remittance (exclusive with text) + {"remittance text", p.Text}, // AT-T009 unstructured remittance + {"beneficiary-to-originator info", p.Info}, // the labels match tooLong's, so every error names a field the same way } values := make([]string, len(fields)) for i, f := range fields { @@ -119,6 +132,14 @@ func (p Payment) Payload() (string, error) { if !utf8.ValidString(f.value) { return "", fmt.Errorf("%s is not valid UTF-8", f.name) } + // Nothing invisible may enter a payment field. A control character + // (ESC above all) drives the terminal that displays the --details + // verification view; a format character — bidi override, zero-width + // space, BOM — makes a beneficiary name read differently from how it + // is stored. EPC069-12 does not contemplate either in these fields. + if r, kind := invisibleRune(f.value); kind != "" { + return "", fmt.Errorf("%s contains %s U+%04X", f.name, kind, r) + } values[i] = f.value } @@ -150,6 +171,36 @@ func hasCombiningMarks(s string) bool { return false } +// invisibleRune returns the first control (Cc) or format (Cf) rune in s with +// a description for the error message, or kind "" when there is none. +// Whitespace other than CR/LF/TAB is not invisible in this sense: a no-break +// space shows as a space and IBAN normalization strips it anyway. +func invisibleRune(s string) (rune, string) { + for _, r := range s { + switch { + case unicode.IsControl(r): + return r, "a control character" + case unicode.Is(unicode.Cf, r): + return r, "an invisible format character" + } + } + return 0, "" +} + +// asciiUpper upper-cases an identifier that is ASCII by definition — BIC, +// purpose code, IBAN. Anything outside ASCII is refused by name before any +// folding: strings.ToUpper turns U+017F (long s) into S and U+0131 (dotless +// i) into I, so a pasted homoglyph would otherwise be validated — and, for +// the checksum-free BIC, encoded — as a value the user never typed. +func asciiUpper(field, s string) (string, error) { + for _, r := range s { + if r > unicode.MaxASCII { + return "", fmt.Errorf("%s contains a non-ASCII character U+%04X (%q); only A-Z and 0-9 are valid", field, r, r) + } + } + return strings.ToUpper(s), nil +} + // tooLong reports a character-limit violation for a text field. func tooLong(field, s string, limit int) error { n := utf8.RuneCountInString(s) diff --git a/internal/epc/epc_test.go b/internal/epc/epc_test.go index e8db255..b406ac6 100644 --- a/internal/epc/epc_test.go +++ b/internal/epc/epc_test.go @@ -3,6 +3,7 @@ package epc import ( "strings" "testing" + "unicode" ) func TestValidateIBAN(t *testing.T) { @@ -31,6 +32,7 @@ func TestValidateIBAN(t *testing.T) { invalid := map[string]string{ "": "empty", + "GB82WE\u017fT12345698765432": "U+017F (long s) folds to S under ToUpper; a homoglyph must be named, never accepted", "DE02120300000000202052": "wrong check digit", "DE0212030000000020205": "21 chars: the DE length rule (22) rejects it before mod-97 runs", "DE0": "too short to carry check digits", @@ -56,6 +58,58 @@ func TestValidateIBAN(t *testing.T) { if _, err := ValidateIBAN("SA0380000000608010167519"); err == nil || !strings.Contains(err.Error(), "SEPA") { t.Errorf("non-SEPA IBAN: error must name SEPA membership, got %v", err) } + // A non-ASCII letter must be named as such before any case folding: the + // generic ToUpper turns U+017F into S and U+0131 into I, so the IBAN + // would otherwise be validated in a form the user never typed. + if _, err := ValidateIBAN("GB82WE\u017fT12345698765432"); err == nil || !strings.Contains(err.Error(), "U+017F") { + t.Errorf("homoglyph IBAN: error must name the non-ASCII character, got %v", err) + } +} + +// TestPayloadRejectsNonASCIIBIC: BIC and purpose code are ASCII by +// definition and, unlike the IBAN, carry no checksum. The generic ToUpper +// silently folded U+017F to S, so a pasted homoglyph became a different, +// plausible BIC with no warning. +func TestPayloadRejectsNonASCIIBIC(t *testing.T) { + const iban = "DE02120300000000202051" + cases := map[string]Payment{ + "long s in BIC": {Name: "X", IBAN: iban, BIC: "\u017fNPAFRPP"}, + "dotless i in BIC": {Name: "X", IBAN: iban, BIC: "BNPAFRPP\u0131XX"}, + "long s in purpose": {Name: "X", IBAN: iban, Purpose: "\u017fALA"}, + "Kelvin sign in BIC": {Name: "X", IBAN: iban, BIC: "\u212aOBADEFF"}, // U+212A folds to K under ToUpper/ToLower + "fullwidth letter, BIC": {Name: "X", IBAN: iban, BIC: "\uff22NPAFRPP"}, + // An RF claim is folded and mod-97 checked; RF35S12345 passes that + // check, so the long s would have produced a valid reference the + // user never typed. + "long s in RF reference": {Name: "X", IBAN: iban, Ref: "RF35\u017f12345"}, + } + for name, p := range cases { + payload, err := p.Payload() + if err == nil { + t.Errorf("%s: accepted, payload %q", name, payload) + continue + } + if !strings.Contains(err.Error(), "U+") { + t.Errorf("%s: error %q must name the non-ASCII codepoint", name, err) + } + } + // Plain ASCII lower case still folds: the rule is about non-ASCII, not + // about case. + p := Payment{Name: "X", IBAN: iban, BIC: "bnpafrpp", Purpose: "gdds"} + payload, err := p.Payload() + if err != nil { + t.Fatalf("ASCII lower-case BIC and purpose must still be accepted: %v", err) + } + if !strings.Contains(payload, "\nBNPAFRPP\n") || !strings.Contains(payload, "\nGDDS") { + t.Errorf("ASCII folding lost: %q", payload) + } + // A reference that is not an RF claim belongs to an issuer's scheme and + // is never folded, so non-ASCII is legal there and must stay verbatim. + issuer := Payment{Name: "X", IBAN: iban, Ref: "Rechnung Müller 2026"} + payload, err = issuer.Payload() + if err != nil || !strings.Contains(payload, "\nRechnung Müller 2026") { + t.Errorf("non-RF reference with non-ASCII must pass through verbatim: %q, %v", payload, err) + } } func TestNormalizeAmount(t *testing.T) { @@ -471,3 +525,83 @@ func TestValidateBIC(t *testing.T) { t.Error("Payload accepted the placeholder BIC 00000000") } } + +// TestPayloadRejectsInvisibleCharacters: only CR and LF used to be refused, +// so every other control character reached the payload and the --details +// view verbatim (ESC drives the terminal that shows it), and bidi overrides +// or zero-width runs could make a beneficiary name read differently from +// how it is stored. Each rejection names the field and the codepoint. +func TestPayloadRejectsInvisibleCharacters(t *testing.T) { + const iban = "DE02120300000000202051" + cases := map[string]struct { + p Payment + want string // field and codepoint the error must name + }{ + "ESC in name": {Payment{Name: "Alice\x1b[31mEVIL", IBAN: iban}, "beneficiary name contains a control character U+001B"}, + "NUL in text": {Payment{Name: "X", IBAN: iban, Text: "paid\x00"}, "remittance text contains a control character U+0000"}, + "TAB in name": {Payment{Name: "ACME\tGmbH", IBAN: iban}, "U+0009"}, + "BEL in info": {Payment{Name: "X", IBAN: iban, Info: "ring\a"}, "U+0007"}, + "DEL in text": {Payment{Name: "X", IBAN: iban, Text: "x\x7fy"}, "U+007F"}, + "C1 control in name": {Payment{Name: "A\u0085B", IBAN: iban}, "U+0085"}, + "RLO in name": {Payment{Name: "ACME\u202e GmbH", IBAN: iban}, "beneficiary name contains an invisible format character U+202E"}, + "LRI in text": {Payment{Name: "X", IBAN: iban, Text: "invoice\u2066 42"}, "U+2066"}, + "PDI in text": {Payment{Name: "X", IBAN: iban, Text: "42\u2069"}, "U+2069"}, + "zero-width space": {Payment{Name: "X", IBAN: iban, Text: "in\u200bvoice"}, "U+200B"}, + "BOM in info": {Payment{Name: "X", IBAN: iban, Info: "\ufeffinfo"}, "U+FEFF"}, + "zero-width joiner": {Payment{Name: "A\u200dB", IBAN: iban}, "U+200D"}, + "ESC in non-RF ref": {Payment{Name: "X", IBAN: iban, Ref: "INV\x1b[2K"}, "structured reference contains a control character U+001B"}, + "escape in purpose code": {Payment{Name: "X", IBAN: iban, Purpose: "A\x1bB"}, ""}, // rejected by the alphanumeric rule; any error will do + } + for name, tc := range cases { + payload, err := tc.p.Payload() + if err == nil { + t.Errorf("%s: accepted, payload %q", name, payload) + continue + } + if !strings.Contains(err.Error(), tc.want) { + t.Errorf("%s: error %q must name the field and codepoint (%q)", name, err, tc.want) + } + } + + // Visible whitespace and decomposed text stay legal: the rule is about + // characters that do not show, not about anything non-ASCII. + good := map[string]Payment{ + "no-break space in name": {Name: "ACME\u00a0GmbH", IBAN: iban}, + "combining mark in name": {Name: "Mu\u0308ller", IBAN: iban}, + "narrow no-break in text": {Name: "X", IBAN: iban, Text: "1\u202f234"}, + "tab inside the IBAN only": {Name: "X", IBAN: "DE02\t1203 0000 0000 2020 51"}, // whitespace is stripped before the check + } + for name, p := range good { + if _, err := p.Payload(); err != nil { + t.Errorf("%s: unexpected error: %v", name, err) + } + } +} + +// FuzzPayload pins the shape invariant behind the field gate: whatever the +// inputs, an accepted payload has at most twelve LF-separated lines and +// carries no control or format character other than the separators. The +// seeds are the hostile inputs from the 2026-09-21 audit. +func FuzzPayload(f *testing.F) { + const iban = "DE02120300000000202051" + f.Add("ACME GmbH", iban, "", "12,50", "", "", "invoice 42", "") + f.Add("Alice\x1b[31mEVIL", iban, "", "", "", "", "", "") + f.Add("X", iban, "", "", "", "", "paid\x1b[2A\x1b[2Kiban: DE00SPOOFED", "") + f.Add("ACME\u202e GmbH", iban, "", "", "", "", "invoice\u2066 42\u2069\u200b", "\ufeffinfo") + f.Add("X", iban, "BNPAFRPP", "0.01", "GDDS", "RF18539007547034", "", "") + f.Fuzz(func(t *testing.T, name, iban, bic, amount, purpose, ref, text, info string) { + p := Payment{Name: name, IBAN: iban, BIC: bic, Amount: amount, Purpose: purpose, Ref: ref, Text: text, Info: info} + payload, err := p.Payload() + if err != nil { + return + } + if n := strings.Count(payload, "\n"); n > 11 { + t.Fatalf("accepted payload has %d lines, EPC069-12 has 12 fields:\n%q", n+1, payload) + } + for _, r := range payload { + if r != '\n' && (unicode.IsControl(r) || unicode.Is(unicode.Cf, r)) { + t.Fatalf("accepted payload carries U+%04X:\n%q", r, payload) + } + } + }) +} diff --git a/internal/epc/iban.go b/internal/epc/iban.go index 59d22b2..427d156 100644 --- a/internal/epc/iban.go +++ b/internal/epc/iban.go @@ -37,11 +37,16 @@ func ValidateIBAN(iban string) (string, error) { if unicode.IsSpace(r) { return -1 } - return unicode.ToUpper(r) + return r }, iban) if s == "" { return "", fmt.Errorf("IBAN is required") } + // Non-ASCII is named before the case fold; see asciiUpper. + s, err := asciiUpper("IBAN", s) + if err != nil { + return "", err + } if len(s) < 4 { return "", fmt.Errorf("IBAN %q is too short to carry a country code and check digits", s) } diff --git a/main.go b/main.go index c13cf4f..15290a8 100644 --- a/main.go +++ b/main.go @@ -4,13 +4,18 @@ package main import ( + "crypto/rand" + "encoding/hex" + "errors" "flag" "fmt" "io" + "io/fs" "os" "path/filepath" "runtime/debug" "strings" + "unicode" "github.com/bmmmm/epcii/internal/epc" "github.com/bmmmm/epcii/internal/qr" @@ -133,7 +138,7 @@ func printDetails(w io.Writer, payload string, code *qr.Code) { fmt.Fprintf(w, "encoded GiroCode payload (%d bytes, QR version %d, %dx%d modules):\n", len(payload), code.Version(), code.Size(), code.Size()) for i, line := range strings.Split(payload, "\n") { - value := line + value := graphic(line) if value == "" { value = "(empty)" } @@ -141,6 +146,25 @@ func printDetails(w io.Writer, payload string, code *qr.Code) { } } +// graphic renders every rune that has no visible form as its \u escape. The +// details view is the verification aid, so it must never carry a byte that +// can drive the terminal showing it; the payload gate refuses such input, +// and this keeps the view honest even if it did not. +func graphic(s string) string { + var b strings.Builder + for _, r := range s { + switch { + case unicode.IsGraphic(r): + b.WriteRune(r) + case r > 0xFFFF: + fmt.Fprintf(&b, `\U%08X`, r) + default: + fmt.Fprintf(&b, `\u%04X`, r) + } + } + return b.String() +} + // looksLikeFlag reports whether a value has the shape of a CLI flag // (-t, --term); negative numbers like "-5" are not flagged so they reach // the amount validation with a better message. @@ -173,10 +197,25 @@ func writePNG(path string, matrix [][]bool) error { keepMode, existed = info.Mode().Perm(), true } - name := filepath.Join(filepath.Dir(path), fmt.Sprintf(".%s.%d.tmp", filepath.Base(path), os.Getpid())) - f, err := os.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o666) - if err != nil { - return err + // The scratch name carries a random suffix. A predictable one (the pid, + // until 2026-09) let a file planted under that name block every write + // into the directory with "file exists" and marked the close-to-rename + // window in a shared, non-sticky directory. O_EXCL still refuses to + // follow a planted symlink; a name collision simply gets a fresh suffix. + var ( + f *os.File + name string + ) + for attempt := 0; ; attempt++ { + name = filepath.Join(filepath.Dir(path), "."+filepath.Base(path)+"."+randomSuffix()+".tmp") + var err error + f, err = os.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o666) + if err == nil { + break + } + if !errors.Is(err, fs.ErrExist) || attempt >= 16 { + return err + } } fail := func(err error) error { os.Remove(name) @@ -200,6 +239,15 @@ func writePNG(path string, matrix [][]bool) error { return nil } +// randomSuffix returns 16 hex characters from the system's random source for +// the PNG scratch name. crypto/rand never fails on the supported platforms +// (it panics rather than returning weak bytes since Go 1.24). +func randomSuffix() string { + var b [8]byte + rand.Read(b[:]) + return hex.EncodeToString(b[:]) +} + // versionString reports the release version. An -ldflags override wins; // otherwise the module version from the build info is used, which is the // requested version for `go install module@version` and a VCS-stamped diff --git a/main_test.go b/main_test.go index 05f9bc2..771c2d6 100644 --- a/main_test.go +++ b/main_test.go @@ -3,12 +3,15 @@ package main import ( "bytes" "errors" + "fmt" "io" "os" "path/filepath" "runtime" "strings" "testing" + + "github.com/bmmmm/epcii/internal/qr" ) func runCLI(t *testing.T, args ...string) (code int, stdout, stderr string) { @@ -106,6 +109,50 @@ func TestRunDetails(t *testing.T) { } } +// TestRunDetailsRejectsEscape: --details is the verification aid, so a +// field carrying an ANSI escape must be refused before anything is printed, +// and no raw ESC byte may reach stderr (on a terminal it would rewrite the +// lines above it — e.g. the iban: line). +func TestRunDetailsRejectsEscape(t *testing.T) { + code, out, errOut := runCLI(t, + "--name", "Alice\x1b[31mEVIL", "--iban", "DE02120300000000202051", "--details") + if code != 2 { + t.Errorf("exit %d, want 2", code) + } + if out != "" { + t.Error("stdout must stay empty when a field is refused") + } + if strings.ContainsRune(errOut, 0x1b) { + t.Errorf("a raw ESC byte reached stderr: %q", errOut) + } + if !strings.Contains(errOut, "U+001B") { + t.Errorf("the refusal must name the codepoint: %q", errOut) + } +} + +// TestPrintDetailsEscapesNonGraphic is the belt to the gate's braces: even +// with a payload the gate did not see, the details view renders anything +// that cannot be displayed as its \u escape instead of the raw byte. +func TestPrintDetailsEscapesNonGraphic(t *testing.T) { + payload := "BCD\n002\n1\nSCT\n\nAlice\x1b[31mEVIL\u202E\nDE02120300000000202051" + code, err := qr.EncodeM([]byte(payload)) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + printDetails(&buf, payload, code) + got := buf.String() + if strings.ContainsAny(got, "\x1b\u202E") { + t.Errorf("details view carries a raw non-graphic character:\n%q", got) + } + if !strings.Contains(got, `Alice\u001B[31mEVIL\u202E`) { + t.Errorf("details view must show the escapes readably:\n%s", got) + } + if !strings.Contains(got, "iban: DE02120300000000202051") { + t.Errorf("graphic text must pass through unchanged:\n%s", got) + } +} + func TestRunPNGAndTerm(t *testing.T) { pngPath := filepath.Join(t.TempDir(), "out.png") code, out, errOut := runCLI(t, @@ -259,6 +306,40 @@ func TestRunPNGFailedWriteKeepsExistingFile(t *testing.T) { assertNoScratchFiles(t, dir) } +// TestRunPNGScratchNameUnpredictable: the scratch file used to be +// ...tmp — guessable, so 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. A decoy under the old name must neither block the write nor be +// touched by it. +func TestRunPNGScratchNameUnpredictable(t *testing.T) { + dir := t.TempDir() + pngPath := filepath.Join(dir, "out.png") + decoy := filepath.Join(dir, fmt.Sprintf(".out.png.%d.tmp", os.Getpid())) + if err := os.WriteFile(decoy, []byte("planted"), 0o644); err != nil { + t.Fatal(err) + } + code, _, errOut := runCLI(t, + "--name", "ACME GmbH", "--iban", "DE02120300000000202051", "--png", pngPath) + if code != 0 { + t.Fatalf("exit %d, stderr: %s — a decoy scratch file must not block the write", code, errOut) + } + data, err := os.ReadFile(pngPath) + if err != nil { + t.Fatal(err) + } + if !bytes.HasPrefix(data, []byte("\x89PNG")) { + t.Error("written file is not a PNG") + } + if got, err := os.ReadFile(decoy); err != nil || string(got) != "planted" { + t.Errorf("the decoy must survive untouched: %q, %v", got, err) + } + if err := os.Remove(decoy); err != nil { + t.Fatal(err) + } + assertNoScratchFiles(t, dir) +} + func TestRunPNGUnwritablePath(t *testing.T) { code, _, errOut := runCLI(t, "--name", "X", "--iban", "DE02120300000000202051", diff --git a/scripts/web-smoke.mjs b/scripts/web-smoke.mjs index a12d0d1..f0301bf 100644 --- a/scripts/web-smoke.mjs +++ b/scripts/web-smoke.mjs @@ -87,12 +87,28 @@ const forbidden = [ [/<(script|link|img|iframe)[^>]+(src|href)=["']https?:/i, 'external resource tag'], [/@import|url\(\s*["']?https?:/i, 'external stylesheet resource'], [/\bimport\s*\(|\bfetch\(\s*["']https?:/, 'dynamic import / cross-origin fetch'], + // The SVG is inserted as a parsed XML node, never as an HTML string: the + // "render.SVG emits only geometry" invariant belongs to another module, + // and a string sink here would turn any future change there into script. + [/\binnerHTML\b|\bouterHTML\s*=|insertAdjacentHTML|document\.write\s*\(/, 'HTML string sink'], ]; for (const name of ['index.html', 'app.js', 'style.css']) { const src = readFileSync(join(root, 'web', name), 'utf8'); const hits = forbidden.filter(([re]) => re.test(src)).map(([, what]) => what); if (hits.length) fail(`web/${name} violates the zero-storage contract: ${hits.join(', ')}`); - else console.log(`ok web/${name} names no storage, address-bar write or external resource`); + else console.log(`ok web/${name} names no storage, address-bar write, external resource or HTML string sink`); +} + +// The download filename must describe the file: rendering is debounced, so +// the form can be ahead of the last encoded result, and a stem read from the +// form would name a file after an IBAN it does not contain. fileStem() may +// only look at that result, never at the DOM. +{ + const src = readFileSync(join(root, 'web', 'app.js'), 'utf8'); + const stem = src.match(/function fileStem\(\)\s*\{[\s\S]*?\n\}/); + if (!stem) fail('web/app.js: fileStem() not found — the filename gate has nothing to check'); + else if (/\$\(|\.value\b|document\./.test(stem[0])) fail('web/app.js: fileStem() reads the form instead of the encoded result'); + else console.log('ok web/app.js fileStem() derives the name from the encoded result'); } process.exit(failures ? 1 : 0); diff --git a/web/app.js b/web/app.js index 8aa37ef..1cb90ed 100644 --- a/web/app.js +++ b/web/app.js @@ -33,6 +33,7 @@ const STR = { load_failed: 'The generator could not be loaded. Your browser needs WebAssembly.', empty: 'Enter at least a name and an IBAN.', bad_link: 'This link was made by a newer version of the page and was not loaded.', + bad_svg: 'The generator returned an image this page cannot show.', dl_svg: 'Download SVG', dl_png: 'Download PNG', copy_link: 'Copy link', @@ -42,7 +43,7 @@ const STR = { share_text: 'Payment QR code', details: 'Encoded payload', details_head: (n, v, s) => `encoded GiroCode payload (${n} bytes, QR version ${v}, ${s}x${s} modules):`, - privacy: 'No server, no cookies, no storage: the code is generated by this page alone. A shared link carries the payment data only in its #fragment, which browsers never send to any server.', + privacy: 'No server, no cookies, no storage: the code is generated by this page alone. A shared link carries the payment data in its #fragment, which browsers do not send in requests — but the link is stored in your browser history like any other URL.', more: 'How this works', }, de: { @@ -63,6 +64,7 @@ const STR = { load_failed: 'Der Generator konnte nicht geladen werden. Der Browser braucht WebAssembly.', empty: 'Mindestens Name und IBAN eingeben.', bad_link: 'Dieser Link stammt von einer neueren Version der Seite und wurde nicht geladen.', + bad_svg: 'Der Generator hat ein Bild geliefert, das diese Seite nicht anzeigen kann.', dl_svg: 'SVG herunterladen', dl_png: 'PNG herunterladen', copy_link: 'Link kopieren', @@ -72,7 +74,7 @@ const STR = { share_text: 'Zahlungs-QR-Code', details: 'Kodierte Nutzdaten', details_head: (n, v, s) => `kodierte GiroCode-Nutzdaten (${n} Bytes, QR-Version ${v}, ${s}x${s} Module):`, - privacy: 'Kein Server, keine Cookies, kein Speicher: der Code entsteht allein auf dieser Seite. Ein geteilter Link trägt die Zahlungsdaten nur im #Fragment, das Browser nie an einen Server senden.', + privacy: 'Kein Server, keine Cookies, kein Speicher: der Code entsteht allein auf dieser Seite. Ein geteilter Link trägt die Zahlungsdaten im #Fragment, das Browser nicht mit Anfragen senden — in der Browser-Chronik landet der Link aber wie jede andere Adresse.', more: 'So funktioniert es', }, }; @@ -143,6 +145,17 @@ function render() { // Fail closed: a stale QR next to new field values must never survive. res = { error: String(e && e.message ? e.message : e) }; } + let svg = null; + if (!res.error) { + // The SVG is parsed as XML and inserted as a node, never assigned as an + // HTML string. render.SVG emits geometry only, but that invariant lives + // in another module; a string sink here would turn any change there + // into script running in this page's origin. + svg = new DOMParser().parseFromString(res.svg, 'image/svg+xml').documentElement; + if (svg.nodeName !== 'svg' || svg.querySelector('parsererror')) { + res = { error: STR[lang].bad_svg }; + } + } if (res.error) { last = null; showError(res.error); @@ -153,8 +166,7 @@ function render() { } last = res; showError(''); - // Safe: render.SVG emits only numeric path data, never user input. - els.qr.innerHTML = res.svg; + els.qr.replaceChildren(document.importNode(svg, true)); els.qr.hidden = false; showDetails(res); els.detailsBox.hidden = false; @@ -213,8 +225,12 @@ function guardAmountInput(e) { // --- downloads +// The stem comes from the payload that was actually encoded, never from the +// form: rendering is debounced, so the IBAN field may already be ahead of +// `last`, and the file must not be named after an IBAN it does not contain. +// Payload line 7 is the IBAN (see PAYLOAD_FIELDS). function fileStem() { - const iban = $('iban').value.replace(/\s+/g, '').toUpperCase(); + const iban = last ? last.payload.split('\n')[6] || '' : ''; return /^[A-Z0-9]+$/.test(iban) ? 'epc-' + iban : 'epc'; } diff --git a/web/index.html b/web/index.html index fc8702d..764904b 100644 --- a/web/index.html +++ b/web/index.html @@ -84,7 +84,7 @@

epcii