diff --git a/worker/backup_handler.go b/worker/backup_handler.go index fcb2776eb82..f2edb2e084c 100644 --- a/worker/backup_handler.go +++ b/worker/backup_handler.go @@ -13,6 +13,7 @@ import ( "net/url" "os" "path/filepath" + "strings" "time" "github.com/golang/glog" @@ -76,6 +77,16 @@ func backupName(since uint64, groupId uint32) string { return fmt.Sprintf(backupNameFmt, since, groupId) } +// cleanRelPath anchors path at the root and cleans it so that a leading +// separator or any number of ".." segments can never resolve above a handler's +// root. It mirrors the containment in http.Dir.Open. The "path" field of a +// backup manifest is read back from the backup location, so a manifest planted +// there must not be able to steer reads/writes outside the handler root. +func cleanRelPath(path string) string { + sep := string(filepath.Separator) + return strings.TrimPrefix(filepath.Join(sep, filepath.FromSlash(path)), sep) +} + // UriHandler interface is implemented by URI scheme handlers. // When adding new scheme handles, for example 'azure://', an object will implement // this interface to supply Dgraph with a way to create or load backup files into DB. @@ -161,7 +172,7 @@ func (h *fileHandler) FileExists(path string) bool { return pathExist(h.Joi func (h *fileHandler) Read(path string) ([]byte, error) { return os.ReadFile(h.JoinPath(path)) } func (h *fileHandler) JoinPath(path string) string { - return filepath.Join(h.rootDir, h.prefix, path) + return filepath.Join(h.rootDir, h.prefix, cleanRelPath(path)) } func (h *fileHandler) Stream(path string) (io.ReadCloser, error) { return os.Open(h.JoinPath(path)) @@ -288,7 +299,7 @@ func (h *s3Handler) FileExists(path string) bool { } func (h *s3Handler) JoinPath(path string) string { - return filepath.Join(h.bucketName, h.objectPrefix, path) + return filepath.Join(h.bucketName, h.objectPrefix, cleanRelPath(path)) } func (h *s3Handler) Read(path string) ([]byte, error) { @@ -410,5 +421,5 @@ func (h *s3Handler) Rename(srcPath, dstPath string) error { } func (h *s3Handler) getObjectPath(path string) string { - return filepath.Join(h.objectPrefix, path) + return filepath.Join(h.objectPrefix, cleanRelPath(path)) } diff --git a/worker/backup_handler_test.go b/worker/backup_handler_test.go new file mode 100644 index 00000000000..ee138ca7013 --- /dev/null +++ b/worker/backup_handler_test.go @@ -0,0 +1,63 @@ +/* + * SPDX-FileCopyrightText: © 2017-2025 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package worker + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// escapesRoot reports whether target is outside base. +func escapesRoot(t *testing.T, base, target string) bool { + t.Helper() + rel, err := filepath.Rel(base, target) + require.NoError(t, err) + return rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} + +func TestFileHandlerJoinPathContainment(t *testing.T) { + root := t.TempDir() + h := &fileHandler{rootDir: root + string(filepath.Separator), prefix: "backups"} + base := filepath.Join(root, "backups") + + traversals := []string{ + "../../../../etc/passwd", + "dgraph.20260101.120000.000/../../../../../../etc/shadow", + filepath.Join("..", "..", "secret.txt"), + "/etc/hosts", + } + for _, p := range traversals { + got := h.JoinPath(p) + require.Falsef(t, escapesRoot(t, base, got), + "JoinPath(%q) escaped handler root: %q", p, got) + } + + // A legitimate relative backup path must be untouched. + good := filepath.Join("dgraph.20260101.120000.000", backupName(42, 1)) + require.Equal(t, filepath.Join(base, good), h.JoinPath(good)) +} + +func TestS3HandlerGetObjectPathContainment(t *testing.T) { + h := &s3Handler{objectPrefix: "dgraph/backups"} + + traversals := []string{ + "../../../../etc/passwd", + "dgraph.1/../../../../other/secret", + "/etc/hosts", + } + for _, p := range traversals { + got := h.getObjectPath(p) + require.Falsef(t, strings.HasPrefix(got, ".."), + "getObjectPath(%q) escaped object prefix: %q", p, got) + } + + // A legitimate relative object path must be untouched. + good := filepath.Join("dgraph.1", backupName(42, 1)) + require.Equal(t, filepath.Join("dgraph/backups", good), h.getObjectPath(good)) +}