Files
seaweedfs/weed/s3api/copy_source_decode_test.go
Chris Lu 0345658ea8 [s3] validate indirect filer path inputs (#9931)
* s3: validate indirect filer path inputs

* s3: avoid query parsing on common request path

* filer: scope copy/move source against JWT AllowedPrefixes

maybeCheckJwtAuthorization only checked r.URL.Path, but copy and move read
their source from the cp.from / mv.from query params. A prefix-restricted
token could copy or move data out of a subtree it cannot otherwise reach.
Check every path the request touches, reusing pathHasComponentPrefix so
`..` in the source is collapsed before the prefix match.

* s3: confine iceberg CreateTable location to the catalog bucket

CreateTable derived the metadata bucket and path from the client-supplied
req.Location / req.Name and wrote there directly, so a caller scoped to one
table bucket could place metadata in another bucket (and path.Join collapsed
any `..`). Require the parsed bucket to equal the request's catalog bucket
and reject traversal segments in the table path.

* webdav: clean client path before subFolder confinement

wrappedFs concatenated subFolder + name before the underlying FileSystem
ran path.Clean, so `..` in the request path or COPY/MOVE Destination
resolved across the FilerRootPath confinement boundary. Clean the name as a
rooted path first so traversal segments collapse below subFolder. Only the
non-default -filer.path (non-empty subFolder) setup was affected.

* filer: enforce read-only rule on real write path with destination header

The x-seaweedfs-destination header overrides the path used for storage-rule
matching while the entry is written at r.URL.Path, letting a caller select a
writable rule for a read-only target. When the header is present, also check
the read-only/quota rule against the actual write path.
2026-06-11 21:56:16 -07:00

278 lines
9.6 KiB
Go

package s3api
import (
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"github.com/gorilla/mux"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
)
func TestCopySourceWithExclamationMark(t *testing.T) {
// Reproduce https://github.com/seaweedfs/seaweedfs/issues/8544
testCases := []struct {
name string
rawCopySource string
expectedBucket string
expectedObject string
expectedVersion string
dstBucket string
dstObject string
shouldBeEqual bool
}{
{
name: "encoded exclamation mark - different dest",
rawCopySource: "my-bucket/path%2Fto%2FAnother%20test%21.odt",
expectedBucket: "my-bucket",
expectedObject: "path/to/Another test!.odt",
dstBucket: "my-bucket",
dstObject: "path/to/Hello.odt",
shouldBeEqual: false,
},
{
name: "unencoded exclamation mark - different dest",
rawCopySource: "my-bucket/path/to/Another%20test!.odt",
expectedBucket: "my-bucket",
expectedObject: "path/to/Another test!.odt",
dstBucket: "my-bucket",
dstObject: "path/to/Hello.odt",
shouldBeEqual: false,
},
{
name: "encoded exclamation mark - same dest",
rawCopySource: "my-bucket/path%2Fto%2FAnother%20test%21.odt",
expectedBucket: "my-bucket",
expectedObject: "path/to/Another test!.odt",
dstBucket: "my-bucket",
dstObject: "path/to/Another test!.odt",
shouldBeEqual: true,
},
{
name: "encoded path with versionId",
rawCopySource: "my-bucket/path%2Fto%2FAnother%20test%21.odt?versionId=abc123",
expectedBucket: "my-bucket",
expectedObject: "path/to/Another test!.odt",
expectedVersion: "abc123",
dstBucket: "my-bucket",
dstObject: "path/to/Hello.odt",
shouldBeEqual: false,
},
{
name: "unencoded path with versionId",
rawCopySource: "my-bucket/path/to/Another%20test!.odt?versionId=v2",
expectedBucket: "my-bucket",
expectedObject: "path/to/Another test!.odt",
expectedVersion: "v2",
dstBucket: "my-bucket",
dstObject: "path/to/Another test!.odt",
shouldBeEqual: true,
},
{
name: "plus sign in key with versionId",
rawCopySource: "my-bucket/path/to/file+name.odt?versionId=xyz",
expectedBucket: "my-bucket",
expectedObject: "path/to/file+name.odt",
expectedVersion: "xyz",
dstBucket: "my-bucket",
dstObject: "path/to/file+name.odt",
shouldBeEqual: true,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
cpSrcPath, err := url.PathUnescape(tc.rawCopySource)
if err != nil {
cpSrcPath = tc.rawCopySource
}
srcBucket, srcObject, srcVersionId := pathToBucketObjectAndVersion(tc.rawCopySource, cpSrcPath)
if srcBucket != tc.expectedBucket {
t.Errorf("expected srcBucket=%q, got %q", tc.expectedBucket, srcBucket)
}
if srcObject != tc.expectedObject {
t.Errorf("expected srcObject=%q, got %q", tc.expectedObject, srcObject)
}
if srcVersionId != tc.expectedVersion {
t.Errorf("expected versionId=%q, got %q", tc.expectedVersion, srcVersionId)
}
isEqual := srcBucket == tc.dstBucket && srcObject == tc.dstObject
if isEqual != tc.shouldBeEqual {
t.Errorf("expected comparison result %v, got %v (srcObject=%q, dstObject=%q)",
tc.shouldBeEqual, isEqual, srcObject, tc.dstObject)
}
})
}
}
// TestCopySourceDecodingPlusSign verifies that + in the copy source header
// is treated as a literal plus sign (not a space), matching url.PathUnescape
// behavior. Using url.QueryUnescape would incorrectly convert + to space.
func TestCopySourceDecodingPlusSign(t *testing.T) {
// Key: "path/to/file+name.odt"
// With url.QueryUnescape, literal "+" → " " (WRONG for paths)
// With url.PathUnescape, literal "+" → "+" (CORRECT)
rawCopySource := "my-bucket/path/to/file+name.odt"
// url.QueryUnescape would give "file name.odt" — WRONG
queryDecoded, _ := url.QueryUnescape(rawCopySource)
if queryDecoded == rawCopySource {
t.Fatal("QueryUnescape should have changed + to space")
}
// url.PathUnescape preserves literal "+" — CORRECT
pathDecoded, _ := url.PathUnescape(rawCopySource)
if pathDecoded != rawCopySource {
t.Fatalf("PathUnescape should have preserved +, got %q", pathDecoded)
}
_, srcObject, _ := pathToBucketObjectAndVersion(rawCopySource, pathDecoded)
if srcObject != "path/to/file+name.odt" {
t.Errorf("expected srcObject=%q, got %q", "path/to/file+name.odt", srcObject)
}
}
// TestCopySourceRejectsTraversal verifies `.`/`..` segments in a copy source
// are rejected so the source stays within the parsed bucket.
func TestCopySourceRejectsTraversal(t *testing.T) {
tests := []struct {
name string
rawCopySource string
wantErr bool
}{
{"clean source passes", "bucket-a/dir/key", false},
{"leading slash source passes", "/bucket-a/dir/key", false},
{"source with versionId passes", "bucket-a/dir/key?versionId=v1", false},
{"dotdot escapes bucket", "bucket-a/../bucket-secret/flag", true},
{"leading slash dotdot escapes bucket", "/bucket-a/../bucket-secret/flag", true},
{"percent-encoded dotdot escapes bucket", "bucket-a/%2e%2e/bucket-secret/flag", true},
{"encoded slash dotdot escapes bucket", "bucket-a/..%2fbucket-secret/flag", true},
{"backslash dotdot escapes bucket", "bucket-a/..\\bucket-secret/flag", true},
{"nested dotdot escapes bucket", "bucket-a/dir/../../bucket-secret/flag", true},
{"bare dot segment rejected", "bucket-a/./key", true},
{"dotdot bucket rejected", "../bucket-secret/flag", true},
{"dotdot with versionId escapes bucket", "bucket-a/../bucket-secret/flag?versionId=v1", true},
{"versionId encoded slash rejected", "bucket-a/dir/key?versionId=v1%2F..%2Fsecret", true},
{"versionId backslash rejected", "bucket-a/dir/key?versionId=v1%5C..%5Csecret", true},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
cpSrcPath, err := url.PathUnescape(tc.rawCopySource)
if err != nil {
cpSrcPath = tc.rawCopySource
}
srcBucket, srcObject, srcVersionId := pathToBucketObjectAndVersion(tc.rawCopySource, cpSrcPath)
gotErr := validateCopySource(cpSrcPath, srcBucket, srcObject, srcVersionId) != nil
if gotErr != tc.wantErr {
t.Errorf("ValidateCopySource(%q) error = %v, want %v (parsed bucket=%q object=%q)",
tc.rawCopySource, gotErr, tc.wantErr, srcBucket, srcObject)
}
})
}
}
// TestCopySourceRoutingWithSpecialChars tests that mux variable extraction
// correctly handles special characters like ! (%21) in both the URL path
// and the X-Amz-Copy-Source header.
func TestCopySourceRoutingWithSpecialChars(t *testing.T) {
testCases := []struct {
name string
dstURL string // URL for the PUT request (destination)
copySource string // X-Amz-Copy-Source header value
expectSameKey bool // whether srcObject should equal dstObject
}{
{
name: "different keys, source has encoded !",
dstURL: "/my-bucket/path/to/Hello.odt",
copySource: "my-bucket/path%2Fto%2FAnother%20test%21.odt",
expectSameKey: false,
},
{
name: "same key with !, source encoded, dest unencoded",
dstURL: "/my-bucket/path/to/Another%20test%21.odt",
copySource: "my-bucket/path%2Fto%2FAnother%20test%21.odt",
expectSameKey: true,
},
{
name: "same key with !, both percent-encoded differently",
dstURL: "/my-bucket/path/to/Another%20test!.odt",
copySource: "my-bucket/path%2Fto%2FAnother%20test%21.odt",
expectSameKey: true,
},
{
name: "key with + sign, source has literal +",
dstURL: "/my-bucket/path/to/file+name.odt",
copySource: "my-bucket/path/to/file+name.odt",
expectSameKey: true,
},
{
name: "key with + sign, source has %2B",
dstURL: "/my-bucket/path/to/file+name.odt",
copySource: "my-bucket/path/to/file%2Bname.odt",
expectSameKey: true,
},
{
name: "lowercase %2f in copy source",
dstURL: "/my-bucket/path/to/Hello.odt",
copySource: "my-bucket/path%2fto%2fAnother%20test.odt",
expectSameKey: false,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
var capturedDstBucket, capturedDstObject string
var capturedSrcBucket, capturedSrcObject string
router := mux.NewRouter().SkipClean(true)
bucket := router.PathPrefix("/{bucket}").Subrouter()
bucket.Methods(http.MethodPut).Path("/{object:(?s).+}").
HeadersRegexp("X-Amz-Copy-Source", `(?i).*?(\/|%2F).*?`).
HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
capturedDstBucket, capturedDstObject = s3_constants.GetBucketAndObject(r)
rawCopySource := r.Header.Get("X-Amz-Copy-Source")
cpSrcPath, err := url.PathUnescape(rawCopySource)
if err != nil {
cpSrcPath = rawCopySource
}
capturedSrcBucket, capturedSrcObject, _ = pathToBucketObjectAndVersion(rawCopySource, cpSrcPath)
if capturedSrcBucket == capturedDstBucket && capturedSrcObject == capturedDstObject {
w.WriteHeader(http.StatusBadRequest)
fmt.Fprintf(w, "ErrInvalidCopyDest")
} else {
w.WriteHeader(http.StatusOK)
fmt.Fprintf(w, "OK")
}
})
req, _ := http.NewRequest("PUT", tc.dstURL, nil)
req.Header.Set("X-Amz-Copy-Source", tc.copySource)
rr := httptest.NewRecorder()
router.ServeHTTP(rr, req)
if tc.expectSameKey {
if rr.Code != http.StatusBadRequest {
t.Errorf("expected same key detection (400), got %d; src=%q dst=%q",
rr.Code, capturedSrcObject, capturedDstObject)
}
} else {
if rr.Code != http.StatusOK {
t.Errorf("expected different keys (200), got %d; src=%q dst=%q",
rr.Code, capturedSrcObject, capturedDstObject)
}
}
})
}
}