TheArchive/tailscale
1
0
Fork 0
mirror of https://github.com/tailscale/tailscale.git synced 2025-02-18 02:48:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

client/web: ensure path prefix has a leading slash

Browse Source 
This is simply an extra check to prevent hypothetical issues if a prefix
such as `--prefix="javascript:alert(1)"` was provided.  This isn't
really necessary since the prefix is a configuration flag provided by
the device owner, not user input.  But it does enforce that we are
always interpreting the provided value as a path relative to the root.

Fixes: tailscale/corp#16268

Signed-off-by: Will Norris <will@tailscale.com>
This commit is contained in:
Will Norris 2024-01-08 11:18:16 -08:00 committed by Will Norris
parent e26ee6952f
commit 569b91417f
2 changed files with 52 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
client/web/web.go
View File

@ -176,11 +176,12 @@ func NewServer(opts ServerOpts) (s *Server, err error) {
waitAuthURL: opts.WaitAuthURL,
}
if opts.PathPrefix != "" {
// In enforcePrefix, we add the necessary leading '/'. If we did not
// strip 1 or more leading '/'s here, we would end up redirecting
// clients to e.g. //example.com (a schema-less URL that points to
// another site). See https://github.com/tailscale/corp/issues/16268.
s.pathPrefix = strings.TrimLeft(path.Clean(opts.PathPrefix), "/\\")
// Enforce that path prefix always has a single leading '/'
// so that it is treated as a relative URL path.
// We strip multiple leading '/' to prevent schema-less offsite URLs like "//example.com".
//
// See https://github.com/tailscale/corp/issues/16268.
s.pathPrefix = "/" + strings.TrimLeft(path.Clean(opts.PathPrefix), "/\\")
}
if s.mode == ManageServerMode {
if opts.NewAuthURL == nil {

63
client/web/web_test.go
View File

@ -939,36 +939,65 @@ func TestServeAPIAuthMetricLogging(t *testing.T) {
}
}
func TestNoOffSiteRedirect(t *testing.T) {
options := ServerOpts{
Mode: LoginServerMode,
// Emulate the admin using a --prefix option with leading slashes:
PathPrefix: "//evil.example.com/goat",
CGIMode: true,
}
s, err := NewServer(options)
if err != nil {
t.Error(err)
}
// TestPathPrefix tests that the provided path prefix is normalized correctly.
// If a leading '/' is missing, one should be added.
// If multiple leading '/' are present, they should be collapsed to one.
// Additionally verify that this prevents open redirects when enforcing the path prefix.
func TestPathPrefix(t *testing.T) {
tests := []struct {
name string
target string
wantHandled bool
prefix string
wantPrefix string
wantLocation string
}{
{
name: "no-leading-slash",
prefix: "javascript:alert(1)",
wantPrefix: "/javascript:alert(1)",
wantLocation: "/javascript:alert(1)/",
},
{
name: "2-slashes",
target: "http://localhost//evil.example.com/goat",
prefix: "//evil.example.com/goat",
// We must also get the trailing slash added:
wantPrefix: "/evil.example.com/goat",
wantLocation: "/evil.example.com/goat/",
},
{
name: "absolute-url",
prefix: "http://evil.example.com",
// We must also get the trailing slash added:
wantPrefix: "/http:/evil.example.com",
wantLocation: "/http:/evil.example.com/",
},
{
name: "double-dot",
prefix: "/../.././etc/passwd",
// We must also get the trailing slash added:
wantPrefix: "/etc/passwd",
wantLocation: "/etc/passwd/",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
options := ServerOpts{
Mode: LoginServerMode,
PathPrefix: tt.prefix,
CGIMode: true,
}
s, err := NewServer(options)
if err != nil {
t.Error(err)
}
// verify provided prefix was normalized correctly
if s.pathPrefix != tt.wantPrefix {
t.Errorf("prefix was not normalized correctly; want=%q, got=%q", tt.wantPrefix, s.pathPrefix)
}
s.logf = t.Logf
r := httptest.NewRequest(httpm.GET, tt.target, nil)
r := httptest.NewRequest(httpm.GET, "http://localhost/", nil)
w := httptest.NewRecorder()
s.ServeHTTP(w, r)
res := w.Result()
@ -976,7 +1005,7 @@ func TestNoOffSiteRedirect(t *testing.T) {
location := w.Header().Get("Location")
if location != tt.wantLocation {
t.Errorf("request(%q) got wrong location; want=%q, got=%q", tt.target, tt.wantLocation, location)
t.Errorf("request got wrong location; want=%q, got=%q", tt.wantLocation, location)
}
})
}