diff --git a/drive/driveimpl/drive_test.go b/drive/driveimpl/drive_test.go index 20b179511..e7dd83291 100644 --- a/drive/driveimpl/drive_test.go +++ b/drive/driveimpl/drive_test.go @@ -133,6 +133,71 @@ func TestPermissions(t *testing.T) { } } +// TestMissingPaths verifies that the fileserver running at localhost +// correctly handles paths with missing required components. +// +// Expected path format: +// http://localhost:[PORT]//[/] +func TestMissingPaths(t *testing.T) { + s := newSystem(t) + + fileserverAddr := s.addRemote(remote1) + s.addShare(remote1, share11, drive.PermissionReadWrite) + + client := &http.Client{ + Transport: &http.Transport{DisableKeepAlives: true}, + } + addr := strings.Split(fileserverAddr, "|")[1] + secretToken := strings.Split(fileserverAddr, "|")[0] + + testCases := []struct { + name string + path string + wantStatus int + }{ + { + name: "empty path", + path: "", + wantStatus: http.StatusForbidden, + }, + { + name: "single slash", + path: "/", + wantStatus: http.StatusForbidden, + }, + { + name: "only token", + path: "/" + secretToken, + wantStatus: http.StatusBadRequest, + }, + { + name: "token with trailing slash", + path: "/" + secretToken + "/", + wantStatus: http.StatusBadRequest, + }, + { + name: "token and invalid share", + path: "/" + secretToken + "/nonexistentshare", + wantStatus: http.StatusNotFound, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + u := fmt.Sprintf("http://%s%s", addr, tc.path) + resp, err := client.Get(u) + if err != nil { + t.Fatalf("unexpected error making request: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != tc.wantStatus { + t.Errorf("got status code %d, want %d", resp.StatusCode, tc.wantStatus) + } + }) + } +} + // TestSecretTokenAuth verifies that the fileserver running at localhost cannot // be accessed directly without the correct secret token. This matters because // if a victim can be induced to visit the localhost URL and access a malicious @@ -704,8 +769,8 @@ func (a *noopAuthenticator) Close() error { return nil } -const lockBody = ` - - - +const lockBody = ` + + + ` diff --git a/drive/driveimpl/fileserver.go b/drive/driveimpl/fileserver.go index ef94b0643..113cb3b44 100644 --- a/drive/driveimpl/fileserver.go +++ b/drive/driveimpl/fileserver.go @@ -142,6 +142,10 @@ func (s *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + if len(parts) < 2 { + w.WriteHeader(http.StatusBadRequest) + return + } r.URL.Path = shared.Join(parts[2:]...) share := parts[1] s.sharesMu.RLock() diff --git a/drive/driveimpl/shared/pathutil.go b/drive/driveimpl/shared/pathutil.go index efa9f5f32..fcadcdd5a 100644 --- a/drive/driveimpl/shared/pathutil.go +++ b/drive/driveimpl/shared/pathutil.go @@ -22,6 +22,9 @@ const ( // CleanAndSplit cleans the provided path p and splits it into its constituent // parts. This is different from path.Split which just splits a path into prefix // and suffix. +// +// If p is empty or contains only path separators, CleanAndSplit returns a slice +// of length 1 whose only element is "". func CleanAndSplit(p string) []string { return strings.Split(strings.Trim(path.Clean(p), sepStringAndDot), sepString) } @@ -38,6 +41,8 @@ func Parent(p string) string { } // Join behaves like path.Join() but also includes a leading slash. +// +// When parts are missing, the result is "/". func Join(parts ...string) string { fullParts := make([]string, 0, len(parts)) fullParts = append(fullParts, sepString) diff --git a/drive/driveimpl/shared/pathutil_test.go b/drive/driveimpl/shared/pathutil_test.go index 662adbd8b..daee69563 100644 --- a/drive/driveimpl/shared/pathutil_test.go +++ b/drive/driveimpl/shared/pathutil_test.go @@ -40,6 +40,7 @@ func TestJoin(t *testing.T) { parts []string want string }{ + {[]string{}, "/"}, {[]string{""}, "/"}, {[]string{"a"}, "/a"}, {[]string{"/a"}, "/a"},