summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorPercy Wegmann <percy@tailscale.com>2024-04-29 11:55:20 -0500
committerPercy Wegmann <percy@tailscale.com>2024-04-29 11:57:45 -0500
commit8fcac10029b15a9a05539efbd1df712f952f77bf (patch)
tree584153325e2440bf7e2e2904eee0cdf7bca892a9
parent74c399483c59e3a3e1c5266086928d730268d576 (diff)
downloadtailscale-ox/corp-19592.tar.xz
tailscale-ox/corp-19592.zip
drive/driveimpl: rewrite text/html Content-Type to text/plainox/corp-19592
This prevents Taildrive from being able to serve HTML content, thereby preventing it from being used to distribute malicious JavaScript. Updates tailscale/corp#19592 Signed-off-by: Percy Wegmann <percy@tailscale.com>
Diffstat
-rw-r--r--drive/driveimpl/drive_test.go42
-rw-r--r--drive/driveimpl/fileserver.go36
2 files changed, 69 insertions, 9 deletions
diff --git a/drive/driveimpl/drive_test.go b/drive/driveimpl/drive_test.go
index 6dc4c7ce5..b66405e36 100644
--- a/drive/driveimpl/drive_test.go
+++ b/drive/driveimpl/drive_test.go
@@ -10,10 +10,12 @@ import (
"log"
"net"
"net/http"
+ "net/url"
"os"
"path"
"path/filepath"
"slices"
+ "strings"
"sync"
"testing"
"time"
@@ -32,8 +34,10 @@ const (
remote2 = `_rem ote$%2`
share11 = `sha re$%11`
share12 = `_sha re$%12`
- file111 = `fi le$%111.txt`
- file112 = `file112.txt`
+ file111 = `fi le$%111.html`
+ file112 = `file112.html`
+
+ contents = "<html>hello world</html>"
)
func init() {
@@ -56,7 +60,7 @@ func TestDirectoryListing(t *testing.T) {
s.checkDirList("remote with one share should contain that share", shared.Join(domain, remote1), share11)
s.addShare(remote1, share12, drive.PermissionReadOnly)
s.checkDirList("remote with two shares should contain both in lexicographical order", shared.Join(domain, remote1), share12, share11)
- s.writeFile("writing file to read/write remote should succeed", remote1, share11, file111, "hello world", true)
+ s.writeFile("writing file to read/write remote should succeed", remote1, share11, file111, contents, true)
s.checkDirList("remote share should contain file", shared.Join(domain, remote1, share11), file111)
s.addRemote(remote2)
@@ -78,9 +82,13 @@ func TestFileManipulation(t *testing.T) {
s.addRemote(remote1)
s.addShare(remote1, share11, drive.PermissionReadWrite)
- s.writeFile("writing file to read/write remote should succeed", remote1, share11, file111, "hello world", true)
+ s.writeFile("writing file to read/write remote should succeed", remote1, share11, file111, contents, true)
s.checkFileStatus(remote1, share11, file111)
s.checkFileContents(remote1, share11, file111)
+ // Despite this being an HTML file, Content-Type should be text/plain to
+ // prevent user agents from treating this as an HTML page. This prevents
+ // Taildrive from being used to distribute malicious JavaScript.
+ s.checkContentType(remote1, share11, file111, "text/plain")
s.renameFile("renaming file across shares should fail", remote1, share11, file111, share12, file112, false)
@@ -88,9 +96,9 @@ func TestFileManipulation(t *testing.T) {
s.checkFileContents(remote1, share11, file112)
s.addShare(remote1, share12, drive.PermissionReadOnly)
- s.writeFile("writing file to read-only remote should fail", remote1, share12, file111, "hello world", false)
- s.writeFile("writing file to non-existent remote should fail", "non-existent", share11, file111, "hello world", false)
- s.writeFile("writing file to non-existent share should fail", remote1, "non-existent", file111, "hello world", false)
+ s.writeFile("writing file to read-only remote should fail", remote1, share12, file111, contents, false)
+ s.writeFile("writing file to non-existent remote should fail", "non-existent", share11, file111, contents, false)
+ s.writeFile("writing file to non-existent share should fail", remote1, "non-existent", file111, contents, false)
}
type local struct {
@@ -124,6 +132,7 @@ func (r *remote) ServeHTTP(w http.ResponseWriter, req *http.Request) {
type system struct {
t *testing.T
local *local
+ baseURL string
client *gowebdav.Client
remotes map[string]*remote
}
@@ -149,11 +158,13 @@ func newSystem(t *testing.T) *system {
}
}()
- client := gowebdav.NewAuthClient(fmt.Sprintf("http://%s", l.Addr()), &noopAuthorizer{})
+ baseURL := fmt.Sprintf("http://%s", l.Addr())
+ client := gowebdav.NewAuthClient(baseURL, &noopAuthorizer{})
client.SetTransport(&http.Transport{DisableKeepAlives: true})
s := &system{
t: t,
local: &local{l: l, fs: fs},
+ baseURL: baseURL,
client: client,
remotes: make(map[string]*remote),
}
@@ -275,6 +286,21 @@ func (s *system) checkFileContents(remoteName, shareName, name string) {
}
}
+func (s *system) checkContentType(remoteName, shareName, name, expectedContentType string) {
+ client := http.Client{
+ Transport: &http.Transport{DisableKeepAlives: true},
+ }
+ p := pathTo(remoteName, shareName, name)
+ resp, err := client.Head(fmt.Sprintf("%s/%s", s.baseURL, url.PathEscape(p)))
+ if err != nil {
+ s.t.Fatalf("unable to check content type: %s", err)
+ }
+ actual := resp.Header.Get("Content-Type")
+ if !strings.Contains(actual, expectedContentType) {
+ s.t.Errorf("%q has wrong Content-Type \nwant: %q\nhave: %q", p, expectedContentType, actual)
+ }
+}
+
func (s *system) checkDirList(label string, path string, want ...string) {
got, err := s.client.ReadDir(path)
if err != nil {
diff --git a/drive/driveimpl/fileserver.go b/drive/driveimpl/fileserver.go
index 73fbd37bb..87af3bab7 100644
--- a/drive/driveimpl/fileserver.go
+++ b/drive/driveimpl/fileserver.go
@@ -6,6 +6,7 @@ package driveimpl
import (
"net"
"net/http"
+ "strings"
"sync"
"github.com/tailscale/xnet/webdav"
@@ -107,9 +108,42 @@ func (s *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
return
}
- h.ServeHTTP(w, r)
+
+ // Rewrite text/html content type to text/plain, to prevent stored XSS
+ // vulnerabilities. This has the effect of not being able to serve HTML
+ // content via Taildrive, which is fine because this is a file sharing, not
+ // web serving, tool.
+ rw := &contentTypeRewritingResponseWriter{
+ ResponseWriter: w,
+ }
+ h.ServeHTTP(rw, r)
}
func (s *FileServer) Close() error {
return s.l.Close()
}
+
+var contentTypeRewrites = map[string]string{
+ "text/html": "text/plain",
+}
+
+// contentTypeRewritingResponseWriter rewrites content types according to the
+// above contentTypeRewrites map.
+type contentTypeRewritingResponseWriter struct {
+ http.ResponseWriter
+}
+
+func (rw *contentTypeRewritingResponseWriter) Header() http.Header {
+ result := rw.ResponseWriter.Header()
+ contentTypes := result.Values("Content-Type")
+ if len(contentTypes) > 0 {
+ for i, contentType := range contentTypes {
+ for from, to := range contentTypeRewrites {
+ contentType = strings.ReplaceAll(contentType, from, to)
+ }
+ contentTypes[i] = contentType
+ }
+ result["Content-Type"] = contentTypes
+ }
+ return result
+}
Copyright © 2025 - 2026 Wayne Cole. WTFPL. Attribution appreciated. No warranty. Not liable for bodily damages.