mirrors/versitygw
1
0
Fork 0
mirror of https://github.com/versity/versitygw.git synced 2026-08-21 14:46:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: copy-source parsing mismatch that could bypass path validation

Browse Source 
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.
This commit is contained in:
Ben McClelland
2026-07-03 13:49:59 -07:00
parent 7416c53ba6
commit 67aaa80d99
4 changed files with 77 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/common.go
+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
backend/common_test.go
+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%",
tests/integration/group-tests.go
+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,
tests/integration/versioning.go
+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 {