From 67aaa80d999f225724c90f8f9f562e4163075580 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Thu, 2 Jul 2026 21:40:37 -0700 Subject: [PATCH] fix: copy-source parsing mismatch that could bypass path validation The copy-source header was being interpreted differently by request validation and the backend parser. An encoded ?versionId= separator could be decoded at one layer but treated as part of the object path at another, which opened a path traversal risk on filesystem-backed copy operations. Align backend copy-source parsing with validation by decoding the header before splitting the versionId suffix, so both layers derive the same bucket, object, and versionId values. This closes the traversal path and adds regression coverage for encoded copy-source inputs. Reported by 5ud0 / Tarmo Technologies. --- backend/common.go | 28 ++++++---------- backend/common_test.go | 8 +++++ tests/integration/group-tests.go | 4 +++ tests/integration/versioning.go | 55 ++++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 18 deletions(-) diff --git a/backend/common.go b/backend/common.go index de8d7efd..299735f6 100644 --- a/backend/common.go +++ b/backend/common.go @@ -238,28 +238,20 @@ func ParseCopySource(copySourceHeader string) (string, string, string, error) { copySourceHeader = copySourceHeader[1:] } - // Split the raw header on the versionId query parameter before any - // URL-decoding so that the '?' delimiter is not percent-encoded. - var rawSource, versionId string - i := strings.LastIndex(copySourceHeader, "?versionId=") - if i == -1 { - rawSource = copySourceHeader - } else { - rawSource = copySourceHeader[:i] - versionId = copySourceHeader[i+11:] - } - - // URL-decode the entire source path first so that clients that send the - // bucket/key separator as "%2F" (e.g. AWS .NET SDK v4) are handled - // correctly before we split on a literal '/'. - decoded, err := url.QueryUnescape(rawSource) + // URL-decode the entire header before splitting the versionId suffix so the + // backend interprets encoded separators the same way as controller + // validation. This also preserves encoded bucket/key separators such as + // "%2F" from AWS .NET SDK v4 clients. + decoded, err := url.QueryUnescape(copySourceHeader) if err != nil { - return "", "", "", s3err.GetInvalidArgumentErr(s3err.InvalidArgCopySourceEncoding, rawSource) + return "", "", "", s3err.GetInvalidArgumentErr(s3err.InvalidArgCopySourceEncoding, copySourceHeader) } - srcBucket, srcObject, ok := strings.Cut(decoded, "/") + decodedSource, versionId, _ := strings.Cut(decoded, "?versionId=") + + srcBucket, srcObject, ok := strings.Cut(decodedSource, "/") if !ok { - return "", "", "", s3err.GetInvalidArgumentErr(s3err.InvalidArgCopySourceBucket, rawSource) + return "", "", "", s3err.GetInvalidArgumentErr(s3err.InvalidArgCopySourceBucket, decodedSource) } return srcBucket, srcObject, versionId, nil diff --git a/backend/common_test.go b/backend/common_test.go index a0307471..af398c56 100644 --- a/backend/common_test.go +++ b/backend/common_test.go @@ -252,6 +252,14 @@ func TestParseCopySource(t *testing.T) { wantVersionId: "", wantErr: false, }, + { + name: "encoded versionId separator stays in version id", + copySourceHeader: "bucket/safe%3FversionId%3D..%2f..%2fsecret.txt", + wantBucket: "bucket", + wantObject: "safe", + wantVersionId: "../../secret.txt", + wantErr: false, + }, { name: "invalid URL encoding - incomplete escape", copySourceHeader: "mybucket/object%", diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 5e627c4d..ddfbae1d 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -1143,6 +1143,7 @@ func TestVersioning(ts *TestState) { ts.Run(Versioning_PutObject_success) // CopyObject action ts.Run(Versioning_CopyObject_invalid_versionId) + ts.Run(Versioning_CopyObject_encoded_versionid_separator_invalid_versionId) ts.Run(Versioning_CopyObject_success) ts.Run(Versioning_CopyObject_non_existing_version_id) ts.Run(Versioning_CopyObject_from_an_object_version) @@ -1205,6 +1206,7 @@ func TestVersioning(ts *TestState) { ts.Run(Versioning_Multipart_Upload_success) ts.Run(Versioning_Multipart_Upload_overwrite_an_object) ts.Run(Versioning_UploadPartCopy_invalid_versionId) + ts.Run(Versioning_UploadPartCopy_encoded_versionid_separator_invalid_versionId) ts.Run(Versioning_UploadPartCopy_non_existing_versionId) ts.Run(Versioning_UploadPartCopy_from_an_object_version) // Object lock configuration @@ -2038,6 +2040,7 @@ func GetIntTests() IntTests { "Versioning_PutObject_overwrite_null_versionId_obj": Versioning_PutObject_overwrite_null_versionId_obj, "Versioning_PutObject_success": Versioning_PutObject_success, "Versioning_CopyObject_invalid_versionId": Versioning_CopyObject_invalid_versionId, + "Versioning_CopyObject_encoded_versionid_separator_invalid_versionId": Versioning_CopyObject_encoded_versionid_separator_invalid_versionId, "Versioning_CopyObject_success": Versioning_CopyObject_success, "Versioning_CopyObject_non_existing_version_id": Versioning_CopyObject_non_existing_version_id, "Versioning_CopyObject_from_an_object_version": Versioning_CopyObject_from_an_object_version, @@ -2087,6 +2090,7 @@ func GetIntTests() IntTests { "Versioning_Multipart_Upload_success": Versioning_Multipart_Upload_success, "Versioning_Multipart_Upload_overwrite_an_object": Versioning_Multipart_Upload_overwrite_an_object, "Versioning_UploadPartCopy_invalid_versionId": Versioning_UploadPartCopy_invalid_versionId, + "Versioning_UploadPartCopy_encoded_versionid_separator_invalid_versionId": Versioning_UploadPartCopy_encoded_versionid_separator_invalid_versionId, "Versioning_UploadPartCopy_non_existing_versionId": Versioning_UploadPartCopy_non_existing_versionId, "Versioning_UploadPartCopy_from_an_object_version": Versioning_UploadPartCopy_from_an_object_version, "Versioning_object_lock_not_enabled_on_bucket_creation": Versioning_object_lock_not_enabled_on_bucket_creation, diff --git a/tests/integration/versioning.go b/tests/integration/versioning.go index cdf13cf6..274d5376 100644 --- a/tests/integration/versioning.go +++ b/tests/integration/versioning.go @@ -244,6 +244,31 @@ func Versioning_CopyObject_invalid_versionId(s *S3Conf) error { }, withVersioning(types.BucketVersioningStatusEnabled)) } +func Versioning_CopyObject_encoded_versionid_separator_invalid_versionId(s *S3Conf) error { + testName := "Versioning_CopyObject_encoded_versionid_separator_invalid_versionId" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + dstObj, srcObj := "dst-obj", "src-obj" + + srcObjLen := int64(2345) + _, err := putObjectWithData(srcObjLen, &s3.PutObjectInput{ + Bucket: &bucket, + Key: &srcObj, + }, s3client) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.CopyObject(ctx, &s3.CopyObjectInput{ + Bucket: &bucket, + Key: &dstObj, + CopySource: getPtr(fmt.Sprintf("%v/%v%%3FversionId%%3D..%%2f..%%2fsecret.txt", bucket, srcObj)), + }) + cancel() + return checkApiErr(err, s3err.GetInvalidArgumentErr(s3err.InvalidArgVersionId, "../../secret.txt")) + }, withVersioning(types.BucketVersioningStatusEnabled)) +} + func Versioning_CopyObject_success(s *S3Conf) error { testName := "Versioning_CopyObject_success" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -1903,6 +1928,36 @@ func Versioning_UploadPartCopy_invalid_versionId(s *S3Conf) error { }) } +func Versioning_UploadPartCopy_encoded_versionid_separator_invalid_versionId(s *S3Conf) error { + testName := "Versioning_UploadPartCopy_encoded_versionid_separator_invalid_versionId" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + dstObj, srcObj := "dst-obj", "src-obj" + _, err := putObjectWithData(10, &s3.PutObjectInput{ + Bucket: &bucket, + Key: &srcObj, + }, s3client) + if err != nil { + return err + } + + mp, err := createMp(s3client, bucket, dstObj) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.UploadPartCopy(ctx, &s3.UploadPartCopyInput{ + Bucket: &bucket, + Key: &dstObj, + UploadId: mp.UploadId, + PartNumber: getPtr(int32(1)), + CopySource: getPtr(fmt.Sprintf("%v/%v%%3FversionId%%3D..%%2f..%%2fsecret.txt", bucket, srcObj)), + }) + cancel() + return checkApiErr(err, s3err.GetInvalidArgumentErr(s3err.InvalidArgVersionId, "../../secret.txt")) + }) +} + func Versioning_UploadPartCopy_non_existing_versionId(s *S3Conf) error { testName := "Versioning_UploadPartCopy_non_existing_versionId" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {