Merge pull request #2225 from versity/ben/path-traversal

fix: copy-source parsing mismatch that could bypass path validation
This commit is contained in:
Ben McClelland
2026-07-03 14:06:56 -07:00
committed by GitHub
4 changed files with 77 additions and 18 deletions
+10 -18
View File
@@ -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
+8
View File
@@ -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%",
+4
View File
@@ -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,
+55
View File
@@ -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 {