diff --git a/internal/clientinject/cilus/cilus.go b/internal/clientinject/cilus/cilus.go index 94b1bfd..086cdcc 100644 --- a/internal/clientinject/cilus/cilus.go +++ b/internal/clientinject/cilus/cilus.go @@ -8,6 +8,21 @@ // The compressed length encodes the size of the following blob bytes (UTF-16 // payload + terminal). Payload budgets for in-place PE overwrite count body // bytes only (UTF-16 + terminal), not the length prefix. +// +// # In-place PE overwrite contract +// +// When replacing a stock string whose body slot is budgetBytes long: +// +// 1. newBody := EncodeBody(s) (must fit: FitsBudget(s, budgetBytes)) +// 2. Rewrite the compressed length prefix to len(newBody) (not budgetBytes). +// 3. Write EncodeBodyPadded(s, budgetBytes) at the body file offset so any +// residual stock bytes past the real terminal are zeroed and cannot leak. +// +// EncodeBodyPadded is NOT a valid #US body of length budgetBytes when the +// string is shorter than the budget: the terminal sits at len(EncodeBody(s))-1, +// with zeros after it. Callers must always rewrite the length prefix to the +// unpadded body length. DecodeBody on the full padded buffer will fail or +// produce garbage — only DecodeBody(padded[:len(EncodeBody(s))]) is valid. package cilus import ( @@ -67,8 +82,10 @@ func DecodeUserString(blob []byte) (string, error) { } // EncodeBody encodes only the body (UTF-16LE + terminal), with no length -// prefix. Used for in-place overwrite at body_file_offset when the length -// prefix stays the same. Always succeeds for any Go string. +// prefix. Always succeeds for any Go string. +// +// For in-place PE overwrite of a fixed stock slot, use EncodeBodyPadded and +// rewrite the compressed length prefix to len(EncodeBody(s)); see package doc. func EncodeBody(s string) []byte { // utf16.Encode handles surrogates for runes > 0xFFFF. u16 := utf16.Encode([]rune(s)) @@ -132,9 +149,20 @@ func FitsBudget(s string, budgetBytes int) bool { return BodyBudgetBytes(s) <= budgetBytes } -// EncodeBodyPadded encodes s as a #US body and zero-fills to budgetBytes so -// leftover stock characters cannot leak on in-place overwrite. Returns -// ErrBudgetExceeded if the encoded body is longer than budgetBytes, and +// EncodeBodyPadded encodes s as a #US body and zero-fills the remainder of a +// stock slot of size budgetBytes so leftover stock characters cannot leak. +// +// Contract (normative for PE patchers): +// +// - The returned buffer is budgetBytes long: EncodeBody(s) followed by zeros. +// - When len(EncodeBody(s)) < budgetBytes, the buffer is NOT a valid #US body +// of length budgetBytes (terminal is early; zeros are residual orphan bytes). +// - The caller MUST rewrite the compressed length prefix to len(EncodeBody(s)), +// then write this padded buffer at the body file offset. +// - When the new body length equals the stock budget, the length prefix may be +// left unchanged (same body size). +// +// Returns ErrBudgetExceeded if EncodeBody(s) is longer than budgetBytes, and // ErrInvalidBudget if budgetBytes is negative. func EncodeBodyPadded(s string, budgetBytes int) ([]byte, error) { if budgetBytes < 0 { @@ -153,12 +181,23 @@ func EncodeBodyPadded(s string, budgetBytes int) ([]byte, error) { return out, nil } -// terminalByte returns 0x00 if every UTF-16 code unit is ≤ 0x007F, else 0x01. -// Per ECMA-335 II.24.2.4: "a final byte holding a 0 or 1; 1 if any UTF16 -// character has a high byte, or is a surrogate". +// terminalByte returns the ECMA-335 II.24.2.4 #US terminal byte for u16. +// +// The final byte is 1 if and only if any UTF-16 code unit has any bit set in +// its top byte, or its low byte is any of: 0x01–0x08, 0x0E–0x1F, 0x27, 0x2D, +// 0x7F. Otherwise it is 0. +// +// Note: ASCII hyphen ('-', 0x2D) forces terminal 0x01. Stock VATSIM JWT URLs +// containing "fsd-jwt" therefore end with 0x01, not 0x00. Conversely, many +// BMP characters with a non-zero low byte outside the special set (e.g. 'é' +// U+00E9) yield terminal 0x00 when the high byte is clear. func terminalByte(u16 []uint16) byte { for _, c := range u16 { - if c > 0x007F { + lo := byte(c) + if c > 0x00FF || + (lo >= 0x01 && lo <= 0x08) || + (lo >= 0x0E && lo <= 0x1F) || + lo == 0x27 || lo == 0x2D || lo == 0x7F { return 0x01 } } diff --git a/internal/clientinject/cilus/cilus_test.go b/internal/clientinject/cilus/cilus_test.go index 85c58ec..0a545de 100644 --- a/internal/clientinject/cilus/cilus_test.go +++ b/internal/clientinject/cilus/cilus_test.go @@ -64,22 +64,43 @@ func TestRoundTripNonASCII(t *testing.T) { } func TestTerminalByte(t *testing.T) { - // All ASCII → terminal 0x00. - body := EncodeBody("hello") - if body[len(body)-1] != 0x00 { - t.Fatalf("ASCII terminal: got 0x%02x want 0x00", body[len(body)-1]) + // ECMA-335 II.24.2.4: terminal is 1 iff any UTF-16 unit has top-byte bits + // set, or low byte in {0x01–0x08, 0x0E–0x1F, 0x27, 0x2D, 0x7F}. + cases := []struct { + s string + term byte + }{ + {"hello", 0x00}, // pure ASCII, no specials + {"", 0x00}, + {"https://auth.vatsim.net/api/fsd-jwt", 0x01}, // hyphen 0x2D in fsd-jwt + {"https://voice1.vatsim.net", 0x00}, // no special low bytes + {"http://fsd.vatsim.net/", 0x00}, + {"a-b", 0x01}, // hyphen 0x2D + {"a'b", 0x01}, // apostrophe 0x27 + {"\x7f", 0x01}, // DEL + {"\x01", 0x01}, + {"\x08", 0x01}, + {"\x0e", 0x01}, + {"\x1f", 0x01}, + {"héllo", 0x00}, // U+00E9: high clear, low 0xE9 not special + {"café", 0x00}, + {"\u0080", 0x00}, // high clear, low 0x80 not special + {"\u0100", 0x01}, // high byte set + {"\U0001F600", 0x01}, // surrogate pair → high bytes set } - - // High byte present → terminal 0x01. - body = EncodeBody("héllo") - if body[len(body)-1] != 0x01 { - t.Fatalf("non-ASCII terminal: got 0x%02x want 0x01", body[len(body)-1]) - } - - // Surrogate pair → terminal 0x01. - body = EncodeBody("\U0001F600") - if body[len(body)-1] != 0x01 { - t.Fatalf("surrogate terminal: got 0x%02x want 0x01", body[len(body)-1]) + for _, tc := range cases { + body := EncodeBody(tc.s) + if body[len(body)-1] != tc.term { + t.Errorf("%q: terminal 0x%02x want 0x%02x", tc.s, body[len(body)-1], tc.term) + } + got, err := DecodeBody(body) + if err != nil { + t.Errorf("%q: DecodeBody: %v", tc.s, err) + continue + } + if got != tc.s { + t.Errorf("%q: DecodeBody got %q", tc.s, got) + } } } @@ -176,6 +197,30 @@ func TestEncodeBodyPadded(t *testing.T) { if !errors.Is(err, ErrInvalidBudget) { t.Fatalf("want ErrInvalidBudget, got %v", err) } + + // Footgun: full padded buffer is NOT a valid #US body of budget length + // (terminal is early; zeros follow). DecodeBody on full pad must not + // round-trip to s. Callers must rewrite length prefix to unpadded body len. + if _, err := DecodeBody(padded); err == nil { + // Even length? budget 10 is even → DecodeBody rejects even length. + // Prefer asserting it does not equal s if it somehow decoded. + t.Fatal("DecodeBody(full padded) should fail (even length or bad terminal position)") + } + // Odd budget with residual zeros after terminal: body of "hi" is 5 bytes + // (h,0,i,0,term); pad to 7 → even? 7 is odd. term at index 4, then zeros. + // DecodeBody treats last byte as terminal (0) and chars include trailing + // zero units — not equal to "hi", or terminal mismatch if term was 0x01. + oddPad, err := EncodeBodyPadded("a-b", 9) // body has hyphen → term 0x01; body len 7 + if err != nil { + t.Fatal(err) + } + if len(EncodeBody("a-b")) != 7 { + t.Fatalf("a-b body len %d", len(EncodeBody("a-b"))) + } + // Full pad DecodeBody: last byte is 0 (padding), not the real terminal. + if got, err := DecodeBody(oddPad); err == nil && got == "a-b" { + t.Fatal("DecodeBody(full padded) must not equal original string") + } } func TestCompressedLengthForms(t *testing.T) { @@ -333,12 +378,16 @@ func TestDecodeErrors(t *testing.T) { if _, err := DecodeBody([]byte{0x61, 0x00, 0x02}); !errors.Is(err, ErrInvalidBody) { t.Fatalf("bad terminal: %v", err) } - // Terminal mismatch: non-ASCII char with terminal 0. - // 'é' = U+00E9 → needs terminal 0x01. - bad := []byte{0xE9, 0x00, 0x00} + // Terminal mismatch: high-byte char (U+0100) requires terminal 0x01. + bad := []byte{0x00, 0x01, 0x00} // U+0100 LE + wrong terminal 0x00 if _, err := DecodeBody(bad); !errors.Is(err, ErrInvalidBody) { t.Fatalf("terminal mismatch: %v", err) } + // Hyphen requires terminal 0x01; wrong terminal 0x00. + badHyphen := []byte{0x2D, 0x00, 0x00} + if _, err := DecodeBody(badHyphen); !errors.Is(err, ErrInvalidBody) { + t.Fatalf("hyphen terminal mismatch: %v", err) + } } func TestDecodeIgnoresTrailing(t *testing.T) { diff --git a/internal/clientinject/vpilotconfig/vpilotconfig.go b/internal/clientinject/vpilotconfig/vpilotconfig.go index 8e8f97b..f452262 100644 --- a/internal/clientinject/vpilotconfig/vpilotconfig.go +++ b/internal/clientinject/vpilotconfig/vpilotconfig.go @@ -181,6 +181,7 @@ func Format(cfg *Config) ([]byte, error) { NetworkLogin: loginEnc, NetworkPassword: passEnc, } + // xml.MarshalIndent only fails for unsupported types; xmlRoot is fixed. body, err := xml.MarshalIndent(root, "", " ") if err != nil { return nil, fmt.Errorf("vpilotconfig: marshal: %w", err) diff --git a/internal/clientinject/vpilotconfig/vpilotconfig_test.go b/internal/clientinject/vpilotconfig/vpilotconfig_test.go index 1c6235f..980575b 100644 --- a/internal/clientinject/vpilotconfig/vpilotconfig_test.go +++ b/internal/clientinject/vpilotconfig/vpilotconfig_test.go @@ -6,6 +6,7 @@ import ( "crypto/des" "encoding/base64" "errors" + "fmt" "strings" "testing" ) @@ -23,12 +24,49 @@ func TestDeriveKey(t *testing.T) { if !bytes.Equal(k1[16:24], k1[0:8]) { t.Fatalf("key extension mismatch: %x vs %x", k1[16:24], k1[0:8]) } + // Fixed production golden (MD5 of ConfigGUID || first 8 of that MD5). + // MD5(5575ac09-f2de-4a1e-808b-e3398e17f8bf) = 9bf2bf12df6e3fb45c0db019e1982a10 + wantHex := "9bf2bf12df6e3fb45c0db019e1982a109bf2bf12df6e3fb4" + gotHex := fmt.Sprintf("%x", k1) + if gotHex != wantHex { + t.Fatalf("DeriveKey hex:\n got %s\nwant %s", gotHex, wantHex) + } // TripleDES accepts the key. if _, err := des.NewTripleDESCipher(k1); err != nil { t.Fatal(err) } } +// Production ciphertext goldens (3DES-ECB-PKCS7 + std Base64), cross-checked +// against OpenSSL. Lock vPilot crypto so MD5/key/ECB/PKCS7 cannot soft-pass. +func TestEncryptGoldens(t *testing.T) { + cases := []struct { + plain string + b64 string + }{ + {"http://status.vatsim.net/", "pZ9u441bE4a2NCGgqMxKNvwAIy0qEA+AwXB8c3sV90c="}, + {"", "QV3c4DoqB1Y="}, + {"AUTOMATIC|fsd.connect.vatsim.net", "vN0yvTPHb8iCqOBSDgJOad3JH+XEldSS/AAjLSoQD4BBXdzgOioHVg=="}, + } + for _, tc := range cases { + got, err := Encrypt(tc.plain) + if err != nil { + t.Fatalf("Encrypt(%q): %v", tc.plain, err) + } + if got != tc.b64 { + t.Fatalf("Encrypt(%q):\n got %s\nwant %s", tc.plain, got, tc.b64) + } + // Decrypt golden back to plaintext. + plain, err := Decrypt(tc.b64) + if err != nil { + t.Fatalf("Decrypt golden for %q: %v", tc.plain, err) + } + if plain != tc.plain { + t.Fatalf("Decrypt golden: got %q want %q", plain, tc.plain) + } + } +} + func TestEncryptDecryptRoundTrip(t *testing.T) { cases := []string{ "", diff --git a/scripts/check-hygiene.sh b/scripts/check-hygiene.sh index f977fac..ea2cdec 100755 --- a/scripts/check-hygiene.sh +++ b/scripts/check-hygiene.sh @@ -98,15 +98,15 @@ fi echo "==> Hygiene: no PE binaries / *.exe / *.dll in git" # Fail if .research/ is somehow tracked (gitignored local RE extracts). -research_tracked="$(git ls-files '.research' '.research/*' 2>/dev/null || true)" -if [[ -n "$research_tracked" ]]; then - echo " FAIL: .research/ must not be tracked (gitignored RE extracts only):" - printf ' %s\n' $research_tracked +research_hit=0 +while IFS= read -r -d '' f; do + echo " FAIL: .research/ must not be tracked (gitignored RE extracts only): $f" + research_hit=1 failed=1 -fi +done < <(git ls-files -z -- '.research' '.research/*' 2>/dev/null || true) pe_hit=0 -while IFS= read -r f; do +while IFS= read -r -d '' f; do [[ -z "$f" || ! -f "$f" ]] && continue base="$(basename "$f")" # Extension check (case-insensitive). @@ -118,17 +118,23 @@ while IFS= read -r f; do continue ;; esac + # Skip known text-only extensions before PE magic read (speed; legal posture + # still catches *.exe/*.dll above and MZ on remaining paths). + case "${base}" in + *.go|*.md|*.txt|*.yml|*.yaml|*.json|*.xml|*.html|*.css|*.js|*.ts|*.sh|*.mod|*.sum|*.toml|*.csv|*.svg|*.gitignore|*.editorconfig|Makefile|Dockerfile*|LICENSE*|NOTICE*|*.proto|*.rhai) + continue + ;; + esac # PE magic "MZ" at start of file (Windows PE / DOS stub). - # Skip empty files and non-regular paths already filtered. magic="$(dd if="$f" bs=2 count=1 2>/dev/null | LC_ALL=C od -An -tx1 | tr -d ' \n')" if [[ "$magic" == "4d5a" ]]; then echo " FAIL: PE magic MZ at start of tracked file: $f" pe_hit=1 failed=1 fi -done < <(git ls-files -z 2>/dev/null | tr '\0' '\n') +done < <(git ls-files -z 2>/dev/null || true) -if [[ "$pe_hit" -eq 0 && -z "$research_tracked" ]]; then +if [[ "$pe_hit" -eq 0 && "$research_hit" -eq 0 ]]; then echo " OK" fi diff --git a/scripts/check-import-graph.sh b/scripts/check-import-graph.sh index 6465f47..49499f4 100755 --- a/scripts/check-import-graph.sh +++ b/scripts/check-import-graph.sh @@ -248,6 +248,23 @@ check_no_imports "internal/clientinject" "${MODULE}/internal/clientinject/..." \ "${MODULE}/internal/auth" \ "${MODULE}/internal/serviceapi" +# cmd/openfsd-client — auxiliary Client Setup tool; never FSD/server internals. +# Skips cleanly until the package lands (same pattern as other missing pkgs). +# GUI deps (e.g. Fyne) are third-party and allowed; only hard-forbid server-side +# packages listed below. Tighten to an allowlist when the cmd exists if needed. +check_no_imports "cmd/openfsd-client" "${MODULE}/cmd/openfsd-client/..." \ + "${MODULE}/internal/server" \ + "${MODULE}/internal/web" \ + "${MODULE}/internal/afv" \ + "${MODULE}/internal/db" \ + "${MODULE}/internal/postoffice" \ + "${MODULE}/internal/session" \ + "${MODULE}/internal/cluster" \ + "${MODULE}/internal/sweatbox" \ + "${MODULE}/internal/metar" \ + "${MODULE}/internal/auth" \ + "${MODULE}/internal/serviceapi" + if [[ "$failed" -ne 0 ]]; then echo echo "Import graph check FAILED. See AGENTS.md §2."