Harden handling of untrusted message content

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
 

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 {

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

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",
 	} {

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,16 +191,16 @@ 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)
 		}
 		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)<i>(.*?)</i>`)
 	aTag      = regexp.MustCompile(`(?is)<a\s[^>]*href="([^"]*)"[^>]*>(.*?)</a>`)
 	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 &#27; 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))
+	})
 }

image_test.go

@@ -32,9 +32,24 @@ func TestKittyPlaceholderWidth(t *testing.T) {
 }
 
 func TestCardHTML(t *testing.T) {
-	got := cardHTML(`a<br>b &amp; <font color="#f00">c</font>`)
-	if got != "a\nb & c" {
-		t.Errorf("cardHTML = %q", got)
+	for in, want := range map[string]string{
+		`a<br>b &amp; <font color="#f00">c</font>`:    "a\nb & c",
+		`<a href="file:///etc/passwd">x</a>`:          "x",
+		`<a href="vscode://x">x</a> &#27;]8;;y&#x9b;`: "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)
 	}
 }
 

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

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)
 	}
 }

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