From cc5a189f00efe68ed0ddb32d3237b42549a9f264 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Sun, 26 Jul 2026 12:14:53 +0100 Subject: [PATCH] serve restic: fix path traversal above the served directory GHSA-45pq-889g-fcgh CVE-PENDING A request path beginning with "../" escaped the path the server was started on, letting a client list, read, create, overwrite and delete objects outside it. The check added for CVE-2026-59733 rejected non-canonical paths by comparing them with path.Clean, but path.Clean cannot resolve leading ".." elements in a relative path so it leaves them in place and the comparison comes out equal. Only interior traversal such as "a/../../x" was rejected. Whether a path then escaped depended on the backend: those which join the root with the remote before encoding it - webdav, ftp, sftp, http and memory - resolved the ".." away, while local and s3 encode the dot elements first and were unaffected. A bare "." was accepted for the same reason, which on bucket backends addresses the served directory's own key. Validate with io/fs.ValidPath instead, which rejects ".", ".." and empty elements wherever they appear. The empty path stays valid as the root of the API, and "." is excluded explicitly because ValidPath accepts it as the root of an FS. --- cmd/serve/restic/restic.go | 12 +++- cmd/serve/restic/restic_traversal_test.go | 76 +++++++++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 cmd/serve/restic/restic_traversal_test.go diff --git a/cmd/serve/restic/restic.go b/cmd/serve/restic/restic.go index db7c16586..4a2ed7e16 100644 --- a/cmd/serve/restic/restic.go +++ b/cmd/serve/restic/restic.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + iofs "io/fs" "net" "net/http" "os" @@ -243,9 +244,14 @@ func WithRemote(next http.Handler) http.Handler { urlpath = r.URL.Path } urlpath = strings.Trim(urlpath, "/") - // Reject any non-canonical path, in particular one containing ".." - // traversal elements. - if urlpath != "" && path.Clean(urlpath) != urlpath { + // Reject anything which isn't a canonical relative path free of "." + // and ".." elements. The backends join the path with the Fs root, so + // such elements could otherwise address objects outside it. + // + // The empty path is the root of the API so is allowed. "." is not a + // valid object name here, even though iofs.ValidPath accepts it as + // the root of an FS. + if urlpath != "" && (urlpath == "." || !iofs.ValidPath(urlpath)) { http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) return } diff --git a/cmd/serve/restic/restic_traversal_test.go b/cmd/serve/restic/restic_traversal_test.go new file mode 100644 index 000000000..db1a7092c --- /dev/null +++ b/cmd/serve/restic/restic_traversal_test.go @@ -0,0 +1,76 @@ +package restic + +import ( + "context" + "io" + "net/http" + "strings" + "testing" + "time" + + _ "github.com/rclone/rclone/backend/memory" + "github.com/rclone/rclone/cmd" + "github.com/rclone/rclone/fs" + "github.com/rclone/rclone/fs/operations" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestResticPathTraversal checks that request paths with "." or ".." +// components are rejected before they reach the backend, so they can't be +// joined with the Fs root to reach objects outside the directory served. +// +// The memory backend is used because it resolves ".." by joining paths, +// unlike the local backend which encodes the components as file names. +func TestResticPathTraversal(t *testing.T) { + ctx := context.Background() + + // outside is the parent of the directory served and holds an object + // which must stay unreachable through the server. + outside := cmd.NewFsSrc([]string{":memory:traversal-test"}) + _, err := operations.Rcat(ctx, outside, "outside-secret.txt", io.NopCloser(strings.NewReader("SECRET")), time.Now(), nil) + require.NoError(t, err) + + f := cmd.NewFsSrc([]string{":memory:traversal-test/served-root"}) + _, err = operations.Rcat(ctx, f, "inside.txt", io.NopCloser(strings.NewReader("INSIDE")), time.Now(), nil) + require.NoError(t, err) + + opt := newOpt() + s, err := newServer(ctx, f, &opt) + require.NoError(t, err) + router := s.server.Router() + + // A path inside the served root is unaffected + checkRequest(t, router.ServeHTTP, + newRequest(t, "GET", "/inside.txt", nil), + []wantFunc{wantCode(http.StatusOK), wantBody("INSIDE")}) + + for _, urlpath := range []string{ + "..", + "../", + "../../", + "../outside-secret.txt", + "../../outside-secret.txt", + "%2e%2e/outside-secret.txt", + "../outside-write.txt", + "a/../outside-secret.txt", + "a/../../outside-secret.txt", + ".", + "./inside.txt", + } { + for _, method := range []string{"GET", "HEAD", "POST", "DELETE"} { + t.Run(method+" /"+urlpath, func(t *testing.T) { + req := newRequest(t, method, "/"+urlpath, strings.NewReader("EVIL")) + checkRequest(t, router.ServeHTTP, req, []wantFunc{wantCode(http.StatusBadRequest)}) + }) + } + } + + // The object outside the served root was neither read nor modified, and + // no new object was created next to it. + o, err := outside.NewObject(ctx, "outside-secret.txt") + require.NoError(t, err) + assert.Equal(t, int64(len("SECRET")), o.Size()) + _, err = outside.NewObject(ctx, "outside-write.txt") + assert.Equal(t, fs.ErrorObjectNotFound, err) +}