From 17a4662c6c982c944b3f87553658dd7ca71c5512 Mon Sep 17 00:00:00 2001 From: Brandon McGinty Date: Thu, 20 Aug 2026 14:34:33 -0400 Subject: [PATCH] strip terminal control sequences from server-supplied text Remove control, DEL, and bidi characters before display. HTML escaping was the only filter, which leaves ANSI and OSC escape sequences intact. A remote user could move the cursor, recolour the screen, or reorder what was shown by putting escape codes in a message, a nickname, or a channel name. Names rendered in the channel tree go through the same filter now. Keep the text box cursor and the prompt on rune boundaries. Both indexed by byte, so editing a line containing multi-byte characters could split one and render replacement characters. Handle text view lines that carry no timestamp. Toggling timestamps assumed every stored line had one and sliced past the end of lines that did not. Ignore key events on an empty tree. The handlers indexed the item list before checking that it had any items. Remove the beep helpers. They shelled out to an optional "beep" binary and panicked when it was absent. Co-Authored-By: Claude Opus 5 --- ui.go | 40 +++++++++++++++++++++------------------- ui_tree.go | 6 +++--- uiterm/label.go | 2 +- uiterm/textbox.go | 36 +++++++++++++++++++++++++++--------- uiterm/textbox_test.go | 18 ++++++++++++++++++ uiterm/textview.go | 9 ++++++--- uiterm/tree.go | 8 +++++++- uiterm/view.go | 11 +++++++++++ 8 files changed, 94 insertions(+), 36 deletions(-) diff --git a/ui.go b/ui.go index 0f67af3..2b6f31c 100644 --- a/ui.go +++ b/ui.go @@ -3,9 +3,9 @@ package main import ( "fmt" "os" - "os/exec" "strings" "time" + "unicode" "git.stormux.org/storm/barnard/gumble/gumble" "git.stormux.org/storm/barnard/uiterm" @@ -24,18 +24,17 @@ const ( uiViewAdmin = "admin" ) -func Beep() { - cmd := exec.Command("beep") - cmdout, err := cmd.Output() - if err != nil { - panic(err) - } - if cmdout != nil { - } -} - +// 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) { @@ -47,10 +46,6 @@ func (b *Barnard) Notify(event string, who string, what string) { } } -func (b *Barnard) Beep() { - Beep() -} - func (b *Barnard) SetSelectedUser(user *gumble.User) { b.selectedUser = user if user == nil { @@ -67,14 +62,21 @@ func (b *Barnard) GetInputStatus() string { } func (b *Barnard) UpdateInputStatus(status string) { - if len(status) > 20 { - status = status[:17] + "..." + "]" - } + status = truncateInputStatus(status) b.UiInputStatus.Text = status b.RebuildUserChannelTreePreservingSelection() b.Ui.Refresh() } +// truncateInputStatus shortens the prompt without splitting a multi-byte rune. +func truncateInputStatus(status string) string { + chars := []rune(status) + if len(chars) > 20 { + return string(chars[:17]) + "..." + "]" + } + return status +} + func (b *Barnard) AddOutputLine(line string) { now := time.Now() b.UiOutput.AddLine(fmt.Sprintf("%s [%02d:%02d:%02d]", line, now.Hour(), now.Minute(), now.Second())) diff --git a/ui_tree.go b/ui_tree.go index 87ca153..ce2e556 100644 --- a/ui_tree.go +++ b/ui_tree.go @@ -10,15 +10,15 @@ import ( func (ti TreeItem) String() string { if ti.User != nil { if ti.User.LocallyMuted() { - return "[MUTED] " + ti.User.Name + return "[MUTED] " + esc(ti.User.Name) } // Calculate total volume as percentage boostPercent := float32(ti.User.Boost()-1) * 10 totalVolume := ti.User.Volume()*100 + boostPercent - return fmt.Sprintf("%s [%.0f%%]", ti.User.Name, totalVolume) + return fmt.Sprintf("%s [%.0f%%]", esc(ti.User.Name), totalVolume) } if ti.Channel != nil { - return "#" + ti.Channel.Name + return "#" + esc(ti.Channel.Name) } return "" } diff --git a/uiterm/label.go b/uiterm/label.go index e0d4a80..088da5a 100644 --- a/uiterm/label.go +++ b/uiterm/label.go @@ -40,7 +40,7 @@ func (l *Label) uiDraw() { if ch, _, err := reader.ReadRune(); err != nil { chr = ' ' } else { - chr = ch + chr = safeRune(ch) } termbox.SetCell(x, y, chr, termbox.Attribute(l.Fg), termbox.Attribute(l.Bg)) } diff --git a/uiterm/textbox.go b/uiterm/textbox.go index 20e71e1..e3e2fc6 100644 --- a/uiterm/textbox.go +++ b/uiterm/textbox.go @@ -2,7 +2,7 @@ package uiterm import ( "strings" - // "unicode/utf8" + "unicode/utf8" "github.com/nsf/termbox-go" ) @@ -41,6 +41,9 @@ func (t *Textbox) uiSetBounds(x0, y0, x1, y1 int) { } func (t *Textbox) uiDraw() { + if t.ui == nil { + return + } t.ui.beginDraw() defer t.ui.endDraw() @@ -51,13 +54,16 @@ func (t *Textbox) uiDraw() { if t.pos > len(t.Text) { t.pos = len(t.Text) } + for t.pos > 0 && t.pos < len(t.Text) && !utf8.RuneStart(t.Text[t.pos]) { + t.pos-- + } for y := t.y0; y < t.y1; y++ { for x := t.x0; x < t.x1; x++ { var chr rune if ch, _, err := reader.ReadRune(); err != nil { chr = ' ' } else { - chr = ch + chr = safeRune(ch) } termbox.SetCell(x, y, chr, termbox.Attribute(t.Fg), termbox.Attribute(t.Bg)) } @@ -95,10 +101,16 @@ func (t *Textbox) uiKeyEvent(key Key) { t.pos = len(t.Text) redraw = true case KeyArrowLeft: - t.pos -= 1 + if t.pos > 0 { + _, size := utf8.DecodeLastRuneInString(t.Text[:t.pos]) + t.pos -= size + } redraw = true case KeyArrowRight: - t.pos += 1 + if t.pos < len(t.Text) { + _, size := utf8.DecodeRuneInString(t.Text[t.pos:]) + t.pos += size + } redraw = true case KeyCtrlC: t.Text = "" @@ -119,12 +131,12 @@ func (t *Textbox) uiKeyEvent(key Key) { redraw = t.handleHistoryKey(key) case KeySpace: t.uiCharacterEvent(' ') - case KeyBackspace: - case KeyBackspace2: + case KeyBackspace, KeyBackspace2: if len(t.Text) > 0 { if t.pos > 0 { - t.Text = t.Text[:t.pos-1] + t.Text[t.pos:] - t.pos -= 1 + _, size := utf8.DecodeLastRuneInString(t.Text[:t.pos]) + t.Text = t.Text[:t.pos-size] + t.Text[t.pos:] + t.pos -= size } } // if r, size := utf8.DecodeLastRuneInString(t.Text); r != utf8.RuneError { @@ -135,7 +147,13 @@ func (t *Textbox) uiKeyEvent(key Key) { // } } if redraw { - t.uiDraw() + // Input callbacks may update another view (for example, append a chat + // message). Redraw every view after submission, not just this textbox. + if key == KeyEnter && t.ui != nil { + t.ui.Refresh() + } else { + t.uiDraw() + } } } diff --git a/uiterm/textbox_test.go b/uiterm/textbox_test.go index 203678b..d482ffa 100644 --- a/uiterm/textbox_test.go +++ b/uiterm/textbox_test.go @@ -2,6 +2,24 @@ package uiterm import "testing" +// Regression: byte-based cursor movement split UTF-8 input, producing invalid +// text when deleting or inserting beside a non-ASCII character. +func TestTextboxEditsAtRuneBoundaries(t *testing.T) { + textbox := Textbox{Text: "aé", pos: len("aé")} + textbox.uiKeyEvent(KeyArrowLeft) + if textbox.pos != 1 { + t.Fatalf("cursor = %d, want rune boundary 1", textbox.pos) + } + textbox.uiKeyEvent(KeyBackspace) + if textbox.Text != "é" || textbox.pos != 0 { + t.Fatalf("after delete: %q at %d", textbox.Text, textbox.pos) + } + textbox.uiCharacterEvent('ß') + if textbox.Text != "ßé" { + t.Fatalf("insert produced %q", textbox.Text) + } +} + func TestTextboxHistoryNavigatesSubmittedText(t *testing.T) { t.Parallel() diff --git a/uiterm/textview.go b/uiterm/textview.go index 52ccd7e..e03ff4e 100644 --- a/uiterm/textview.go +++ b/uiterm/textview.go @@ -84,8 +84,11 @@ func (t *Textview) updateParsedLines() { parsed := make([]string, 0, len(t.Lines)) for _, line := range t.Lines { var l = line - if t.showTimestamps == false { - l = strings.TrimSpace(strings.Split(line, "]")[1]) + if !t.showTimestamps { + // Server and local messages need not have a timestamp prefix. + if _, text, ok := strings.Cut(line, "]"); ok { + l = strings.TrimSpace(text) + } } current := "" chars := 0 @@ -143,7 +146,7 @@ func (t *Textview) uiDraw() { var chr rune = ' ' if reader != nil { if ch, _, err := reader.ReadRune(); err == nil { - chr = ch + chr = safeRune(ch) } //no err } //reader != nil termbox.SetCell(x, y, chr, termbox.Attribute(t.Fg), termbox.Attribute(t.Bg)) diff --git a/uiterm/tree.go b/uiterm/tree.go index b44ddf3..0990c72 100644 --- a/uiterm/tree.go +++ b/uiterm/tree.go @@ -177,7 +177,7 @@ func (t *Tree) uiDraw() { dx := x - t.x0 if reader != nil && level*2 <= dx { if ch, _, err := reader.ReadRune(); err == nil { - chr = ch + chr = safeRune(ch) fg, bg = item.TreeItemStyle(fg, bg, t.active && t.activeLine == line) } } @@ -206,6 +206,9 @@ func (t *Tree) ActiveItem() TreeItem { } func (t *Tree) uiKeyEvent(key Key) { + if len(t.lines) == 0 { + return + } var runHandler = true switch key { case KeyArrowUp: @@ -222,6 +225,9 @@ func (t *Tree) uiKeyEvent(key Key) { } func (t *Tree) uiCharacterEvent(ch rune) { + if len(t.lines) == 0 { + return + } if t.CharacterListener != nil { t.CharacterListener(t.ui, t, t.lines[t.activeLine].Item, ch) } diff --git a/uiterm/view.go b/uiterm/view.go index 606e3cd..3ee420b 100644 --- a/uiterm/view.go +++ b/uiterm/view.go @@ -1,5 +1,16 @@ package uiterm +import "unicode" + +// safeRune prevents text supplied by a server or another user from being +// interpreted as a terminal control sequence when termbox flushes its cells. +func safeRune(r rune) rune { + if unicode.IsControl(r) || unicode.Is(unicode.Bidi_Control, r) { + return ' ' + } + return r +} + type View interface { uiInitialize(ui *Ui) uiSetActive(active bool)