From ea27d87fc76e6d4eab464ef6c1b2e2e141075111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20N=C3=B8rgaard?= Date: Sat, 3 Oct 2026 12:59:27 +0200 Subject: [PATCH] Harden handling of untrusted message content Strip C0 and C1 control characters from message text, card fields and desktop notifications, and drop C1 controls from the whole frame, so other people's messages can't inject terminal escape sequences. Card HTML entities unescape one at a time so  can't produce a raw ESC. Only http and https links in cards become clickable, matching openURL. safeName strips leading dots so an attachment can't land as a dotfile, /save creates ~/Downloads instead of falling back to the home directory, and /open downloads into the user cache dir instead of a shared /tmp directory. --- README.md | 2 +- actions.go | 20 +++++++++++------- attach.go | 32 +++++++++++++++++------------ attach_test.go | 3 +++ format.go | 56 +++++++++++++++++++++++++++++++++++++++++--------- image_test.go | 21 ++++++++++++++++--- notify.go | 3 ++- notify_test.go | 2 +- ui.go | 4 ++-- 9 files changed, 105 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index b961578..2a899a0 100644 --- a/README.md +++ b/README.md @@ -208,7 +208,7 @@ While a thread or message is selected, letter keys act on it. Any other key goes | `/logout` | delete the stored token and quit | | `/quit` | quit | -Saved files get the macOS quarantine attribute, so Gatekeeper checks them before they run. `/open` only follows http and https links. +Saved files get the macOS quarantine attribute, so Gatekeeper checks them before they run. Leading dots are stripped from their names, so a file can't land as a hidden dotfile. `/save` creates `~/Downloads` when it's missing. `/open` downloads into mutter's directory in the user cache dir. `/open` only follows http and https links, and only http and https links in cards are clickable. ### Permissions diff --git a/actions.go b/actions.go index 8746725..6bd2842 100644 --- a/actions.go +++ b/actions.go @@ -5,8 +5,6 @@ import ( "context" "fmt" "net/url" - "os" - "path/filepath" "strings" tea "charm.land/bubbletea/v2" @@ -206,8 +204,8 @@ func (m *model) openFile(f file) (string, error) { } return "opened " + f.label + " in the browser", nil } - dir := filepath.Join(os.TempDir(), "mutter") - if err := os.MkdirAll(dir, 0o700); err != nil { + dir, err := openDir() + if err != nil { return "", err } path, err := m.c.saveFile(m.ctx, f, dir) @@ -227,7 +225,11 @@ func (m *model) saveToDownloads(f file) (string, error) { } return "opened " + f.label + " in the browser, Drive files and GIFs can't be saved from here", nil } - path, err := m.c.saveFile(m.ctx, f, downloadsDir()) + dir, err := downloadsDir() + if err != nil { + return "", err + } + path, err := m.c.saveFile(m.ctx, f, dir) if err != nil { return "", err } @@ -238,14 +240,18 @@ func (m *model) saveToDownloads(f file) (string, error) { // messages, and open hands custom schemes to local apps, so only http(s) // passes. func openURL(f file) error { - u, err := url.Parse(f.url) - if err != nil || (u.Scheme != "https" && u.Scheme != "http") { + if !webLink(f.url) { return fmt.Errorf("won't open %s, its link isn't http(s)", f.label) } openBrowser(f.url) return nil } +func webLink(s string) bool { + u, err := url.Parse(s) + return err == nil && (u.Scheme == "https" || u.Scheme == "http") +} + // unreadFrom marks the open space unread from at onward. It holds off // marking the space read again until it's reopened. func (m *model) unreadFrom(at string) tea.Cmd { diff --git a/attach.go b/attach.go index 7e03051..0ce6c9f 100644 --- a/attach.go +++ b/attach.go @@ -52,21 +52,26 @@ func threadFiles(t *thread) []file { return out } -// downloadsDir is where /save puts files. -func downloadsDir() string { +// downloadsDir is where /save puts files. It's never the home directory +// itself, where a file named like a dotfile would run at the next login. +func downloadsDir() (string, error) { home, err := os.UserHomeDir() if err != nil { - return os.TempDir() - } - if d := filepath.Join(home, "Downloads"); isDir(d) { - return d + return "", err } - return home + d := filepath.Join(home, "Downloads") + return d, os.MkdirAll(d, 0o700) } -func isDir(p string) bool { - fi, err := os.Stat(p) - return err == nil && fi.IsDir() +// openDir is where /open puts files before opening them. It's per user, +// because in a shared temp dir another local user could swap the file. +func openDir() (string, error) { + cache, err := os.UserCacheDir() + if err != nil { + return "", err + } + d := filepath.Join(cache, "mutter", "open") + return d, os.MkdirAll(d, 0o700) } // saveFile downloads an uploaded file into dir and returns its path. @@ -95,10 +100,11 @@ func (c *client) saveFile(ctx context.Context, f file, dir string) (string, erro return path, nil } -// safeName keeps a sender-chosen name from escaping the target directory. +// safeName keeps a sender-chosen name from escaping the target directory or +// landing as a hidden dotfile. func safeName(name string) string { - name = filepath.Base(strings.ReplaceAll(name, `\`, "/")) - if name == "." || name == ".." || name == "/" || name == "" { + name = strings.TrimLeft(filepath.Base(strings.ReplaceAll(name, `\`, "/")), ".") + if name == "/" || name == "" { return "attachment" } return name diff --git a/attach_test.go b/attach_test.go index ba323fa..4f731b3 100644 --- a/attach_test.go +++ b/attach_test.go @@ -12,6 +12,9 @@ func TestSafeName(t *testing.T) { "../../.ssh/config": "config", `..\..\evil.exe`: "evil.exe", "..": "attachment", + "...": "attachment", + ".zshenv": "zshenv", + "x/..bash_login": "bash_login", "": "attachment", "dir/": "dir", } { diff --git a/format.go b/format.go index bf30441..86625fc 100644 --- a/format.go +++ b/format.go @@ -6,6 +6,7 @@ import ( "html" "regexp" "strings" + "unicode" "charm.land/lipgloss/v2" "google.golang.org/api/chat/v1" @@ -35,8 +36,32 @@ var inline = []struct { {marker("~"), strikeStyle}, } +// clean drops control characters other than newline and tab from untrusted +// text, so it can't smuggle escape sequences into the terminal. +func clean(s string) string { + return strings.Map(func(r rune) rune { + if unicode.IsControl(r) && r != '\n' && r != '\t' { + return -1 + } + return r + }, s) +} + +// stripC1 drops C1 controls from a whole frame. The renderer filters 7-bit +// sequences but passes C1 through, and names, titles and notices reach the +// frame uncleaned. The styles only emit 7-bit escapes, so none are lost. +func stripC1(s string) string { + return strings.Map(func(r rune) rune { + if r >= 0x80 && r <= 0x9f { + return -1 + } + return r + }, s) +} + // formatText renders Google Chat markup for the terminal. func formatText(s string) string { + s = clean(s) var b strings.Builder for i, block := range strings.Split(s, "```") { if i%2 == 1 { @@ -82,7 +107,7 @@ func messageBody(m *chat.Message, img func(ref string) string, num *int) string } } if len(parts) == 0 && m.FallbackText != "" { - parts = append(parts, m.FallbackText) + parts = append(parts, clean(m.FallbackText)) } for _, a := range m.Attachment { *num++ @@ -132,10 +157,10 @@ func cardText(c *chat.GoogleAppsCardV1Card) string { var lines []string if h := c.Header; h != nil { if h.Title != "" { - lines = append(lines, boldStyle.Render(h.Title)) + lines = append(lines, boldStyle.Render(clean(h.Title))) } if h.Subtitle != "" { - lines = append(lines, dimStyle.Render(h.Subtitle)) + lines = append(lines, dimStyle.Render(clean(h.Subtitle))) } } for _, sec := range c.Sections { @@ -166,8 +191,8 @@ func widgetLines(w *chat.GoogleAppsCardV1Widget) []string { if b := w.ButtonList; b != nil { buttons := make([]string, 0, len(b.Buttons)) for _, btn := range b.Buttons { - label := "[ " + btn.Text + " ]" - if btn.OnClick != nil && btn.OnClick.OpenLink != nil { + label := "[ " + clean(btn.Text) + " ]" + if btn.OnClick != nil && btn.OnClick.OpenLink != nil && webLink(btn.OnClick.OpenLink.Url) { label = linkStyle.Hyperlink(btn.OnClick.OpenLink.Url).Render(label) } buttons = append(buttons, label) @@ -175,7 +200,7 @@ func widgetLines(w *chat.GoogleAppsCardV1Widget) []string { lines = append(lines, strings.Join(buttons, " ")) } if i := w.Image; i != nil { - lines = append(lines, dimStyle.Render("[image: "+cmp.Or(i.AltText, i.ImageUrl)+"]")) + lines = append(lines, dimStyle.Render("[image: "+clean(cmp.Or(i.AltText, i.ImageUrl))+"]")) } if w.Divider != nil { lines = append(lines, dimStyle.Render("───")) @@ -202,17 +227,28 @@ var ( iTag = regexp.MustCompile(`(?is)(.*?)`) aTag = regexp.MustCompile(`(?is)]*href="([^"]*)"[^>]*>(.*?)`) anyTag = regexp.MustCompile(`<[^>]+>`) + entity = regexp.MustCompile(`&#?[0-9A-Za-z]+;?`) ) // cardHTML renders the HTML subset that card text fields accept. Tags other -// than b, i, a and br are dropped and their text kept. +// than b, i, a and br are dropped and their text kept. Only http(s) links +// become clickable, because terminals hand other schemes to local apps. func cardHTML(s string) string { - s = brTag.ReplaceAllString(s, "\n") + s = brTag.ReplaceAllString(clean(s), "\n") s = aTag.ReplaceAllStringFunc(s, func(m string) string { sub := aTag.FindStringSubmatch(m) - return linkStyle.Hyperlink(html.UnescapeString(sub[1])).Render(sub[2]) + href := html.UnescapeString(sub[1]) + if !webLink(href) { + return sub[2] + } + return linkStyle.Hyperlink(href).Render(sub[2]) }) s = bTag.ReplaceAllStringFunc(s, func(m string) string { return boldStyle.Render(bTag.FindStringSubmatch(m)[1]) }) s = iTag.ReplaceAllStringFunc(s, func(m string) string { return italicStyle.Render(iTag.FindStringSubmatch(m)[1]) }) - return html.UnescapeString(anyTag.ReplaceAllString(s, "")) + + // Entities unescape one at a time, so  can't become a raw ESC + // among the styling escapes. + return entity.ReplaceAllStringFunc(anyTag.ReplaceAllString(s, ""), func(e string) string { + return clean(html.UnescapeString(e)) + }) } diff --git a/image_test.go b/image_test.go index 8b22608..ea391a6 100644 --- a/image_test.go +++ b/image_test.go @@ -32,9 +32,24 @@ func TestKittyPlaceholderWidth(t *testing.T) { } func TestCardHTML(t *testing.T) { - got := cardHTML(`a
b & c`) - if got != "a\nb & c" { - t.Errorf("cardHTML = %q", got) + for in, want := range map[string]string{ + `a
b & c`: "a\nb & c", + `x`: "x", + `x ]8;;y›`: "x ]8;;y›", + "a\x1b]52;c;aGk=\x07b\u009b2J": "a]52;c;aGk=b2J", + } { + if got := cardHTML(in); got != want { + t.Errorf("cardHTML(%q) = %q, want %q", in, got, want) + } + } +} + +func TestFormatTextDropsControls(t *testing.T) { + if got := formatText("a\x1b]8;;file:///x\x07b\u009dc\n\td"); got != "a]8;;file:///xbc\n\td" { + t.Errorf("formatText = %q", got) + } + if got := stripC1("a\u009b2J\x1b[1mb"); got != "a2J\x1b[1mb" { + t.Errorf("stripC1 = %q", got) } } diff --git a/notify.go b/notify.go index ee0a7d3..1a630af 100644 --- a/notify.go +++ b/notify.go @@ -7,6 +7,7 @@ import ( "strings" "sync" "time" + "unicode" "golang.org/x/sync/errgroup" "google.golang.org/api/chat/v1" @@ -186,7 +187,7 @@ func shouldNotify(s *chat.SpaceNotificationSetting, dm, mentioned, newThread boo func osc777(title, body string) string { clean := func(s string) string { return strings.Map(func(r rune) rune { - if r == ';' || r < 0x20 || r == 0x7f { + if r == ';' || unicode.IsControl(r) { return ' ' } return r diff --git a/notify_test.go b/notify_test.go index 8719611..e74ddfd 100644 --- a/notify_test.go +++ b/notify_test.go @@ -48,7 +48,7 @@ func TestMentionsAndReadState(t *testing.T) { if got := readStateSpace("users/1/spaces/AAA/spaceReadState"); got != "spaces/AAA" { t.Errorf("readStateSpace = %s", got) } - if got := osc777("a;b", "c\nd"); got != "\x1b]777;notify;a b;c d\x1b\\" { + if got := osc777("a;b", "c\nd\u009ce"); got != "\x1b]777;notify;a b;c d e\x1b\\" { t.Errorf("osc777 = %q", got) } } diff --git a/ui.go b/ui.go index 3e4588d..57596fc 100644 --- a/ui.go +++ b/ui.go @@ -998,12 +998,12 @@ func (m model) View() tea.View { if status == "" { status = dimStyle.Render(hint) } - v := tea.NewView(lipgloss.JoinVertical(lipgloss.Left, + v := tea.NewView(stripC1(lipgloss.JoinVertical(lipgloss.Left, headerStyle.Render(title), body, inputStyle.Render(m.ta.View()), status, - )) + ))) v.AltScreen = true v.ReportFocus = true return v