Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions internal/handlers/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
package httphandlers

import (
"path"
"strings"

"github.com/gin-gonic/gin"
"github.com/google/uuid"
"github.com/poyrazk/thecloud/internal/errors"
Expand Down Expand Up @@ -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.
Comment on lines +67 to +72
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
}
Comment on lines +73 to +90
32 changes: 32 additions & 0 deletions internal/handlers/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
Comment on lines +126 to +139

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)
}
})
}
Loading