diff --git a/client_notification_test.go b/client_notification_test.go index 9eb0f74..501525f 100644 --- a/client_notification_test.go +++ b/client_notification_test.go @@ -6,6 +6,15 @@ import ( "git.stormux.org/storm/barnard/gumble/gumble" ) +// Regression: HTML escaping left terminal control sequences in server text, +// allowing ANSI/OSC sequences to alter the terminal that rendered it. +func TestEscRemovesTerminalControlSequences(t *testing.T) { + got := esc("name\x1b]0;spoof\a\x7f\u202e") + if got != "name]0;spoof" { + t.Fatalf("unsafe terminal text %q", got) + } +} + func TestUserChangeNotification(t *testing.T) { current := &gumble.Channel{ID: 1, Name: "Current"} other := &gumble.Channel{ID: 2, Name: "Other"} diff --git a/fix.txt b/fix.txt index f095897..6208750 100644 --- a/fix.txt +++ b/fix.txt @@ -59,7 +59,7 @@ Priority 0: security and crashers from arbitrary goroutines. Route UI work through a UI-owned event queue, or protect all state and ensure only the UI goroutine calls termbox. -7. Terminal control sequences from server data are rendered +[x] 7. Terminal control sequences from server data are rendered Files: client.go, ui.go, ui_tree.go, admin.go HTML escaping does not remove terminal escape/control sequences. Server supplied messages, names, comments, and channel names are displayed in the diff --git a/ui.go b/ui.go index aa8b820..fe1f48b 100644 --- a/ui.go +++ b/ui.go @@ -5,6 +5,7 @@ import ( "os" "strings" "time" + "unicode" "git.stormux.org/storm/barnard/gumble/gumble" "git.stormux.org/storm/barnard/uiterm" @@ -23,8 +24,17 @@ const ( uiViewAdmin = "admin" ) +// esc makes server-supplied text safe for a terminal as well as for HTML. +// HTML escaping alone leaves ANSI, OSC, DEL, and bidi/control characters able +// to alter terminal state or obscure the displayed text. func esc(str string) string { - return sanitize.HTML(str) + clean := strings.Map(func(r rune) rune { + if r == 0x7f || unicode.IsControl(r) || unicode.Is(unicode.Bidi_Control, r) { + return -1 + } + return r + }, str) + return sanitize.HTML(clean) } func (b *Barnard) Notify(event string, who string, what string) {