diff --git a/internal/handlers/helper.go b/internal/handlers/helper.go index b231ae61d..be00af2eb 100644 --- a/internal/handlers/helper.go +++ b/internal/handlers/helper.go @@ -2,6 +2,9 @@ package httphandlers import ( + "path" + "strings" + "github.com/gin-gonic/gin" "github.com/google/uuid" "github.com/poyrazk/thecloud/internal/errors" @@ -53,5 +56,35 @@ func getBucketAndKeyRequired(c *gin.Context) (bucket, key string, ok bool) { return "", "", false } + if err := validateObjectKey(key); err != nil { + httputil.Error(c, err) + return "", "", false + } + return bucket, key, true } + +// validateObjectKey rejects object keys that could be used for path traversal, +// contain control characters, or would otherwise be unsafe when used as a +// backend object name. Any `..` segment in the raw key is rejected (we do +// not silently collapse it, because the user request explicitly tried to +// reference a parent directory). NUL bytes are rejected outright because +// they can truncate strings in some backends. +func validateObjectKey(key string) error { + if strings.ContainsRune(key, 0x00) { + return errors.New(errors.InvalidInput, "invalid characters in key") + } + + for _, seg := range strings.Split(key, "/") { + if seg == ".." { + return errors.New(errors.InvalidInput, "path traversal in key") + } + } + + // Reject keys whose canonical form would be just the root or empty. + cleaned := path.Clean("/" + strings.TrimPrefix(key, "/")) + if cleaned == "/" { + return errors.New(errors.InvalidInput, "invalid key") + } + return nil +} diff --git a/internal/handlers/helper_test.go b/internal/handlers/helper_test.go index 29363b01b..b381c5f53 100644 --- a/internal/handlers/helper_test.go +++ b/internal/handlers/helper_test.go @@ -120,4 +120,36 @@ func TestGetBucketAndKeyRequired(t *testing.T) { assert.Equal(t, http.StatusBadRequest, w.Code) assert.Contains(t, w.Body.String(), "key is required") }) + + // Regression for #683 — keys containing path traversal segments or NUL + // bytes must be rejected before reaching the storage backend. + t.Run("rejects path traversal", func(t *testing.T) { + badKeys := []string{"../foo", "a/../b", "../../etc/passwd", "..\x00", "/../escape"} + for _, k := range badKeys { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = []gin.Param{ + {Key: "bucket", Value: testBucket}, + {Key: "key", Value: k}, + } + _, _, ok := getBucketAndKeyRequired(c) + assert.Falsef(t, ok, "expected key %q to be rejected", k) + assert.Equal(t, http.StatusBadRequest, w.Code) + } + }) + + t.Run("accepts nested keys", func(t *testing.T) { + goodKeys := []string{"a.txt", "folder/file.png", "deep/nested/object"} + for _, k := range goodKeys { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = []gin.Param{ + {Key: "bucket", Value: testBucket}, + {Key: "key", Value: k}, + } + _, gotKey, ok := getBucketAndKeyRequired(c) + assert.Truef(t, ok, "expected key %q to be accepted", k) + assert.Equal(t, k, gotKey) + } + }) }