Merge pull request #1404 from versity/sis/copy-actions-copy-source-validation

feat: adds copy source validation for x-amz-copy-source header.
This commit is contained in:
Ben McClelland
2025-07-22 14:56:32 -07:00
committed by GitHub
10 changed files with 325 additions and 49 deletions
+1 -1
View File
@@ -40,7 +40,7 @@ func VerifyObjectCopyAccess(ctx context.Context, be backend.Backend, copySource
// Verify source bucket access
srcBucket, srcObject, found := strings.Cut(copySource, "/")
if !found {
return s3err.GetAPIError(s3err.ErrInvalidCopySource)
return s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)
}
// Get source bucket ACL
+1 -1
View File
@@ -230,7 +230,7 @@ func ParseCopySource(copySourceHeader string) (string, string, string, error) {
srcBucket, srcObject, ok := strings.Cut(copySource, "/")
if !ok {
return "", "", "", s3err.GetAPIError(s3err.ErrInvalidCopySource)
return "", "", "", s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)
}
return srcBucket, srcObject, versionId, nil
+1 -1
View File
@@ -3871,7 +3871,7 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput
return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidCopyDest)
}
if input.CopySource == nil {
return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidCopySource)
return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)
}
if input.ExpectedBucketOwner == nil {
return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidRequest)
+8 -11
View File
@@ -19,7 +19,6 @@ import (
"encoding/xml"
"fmt"
"io"
"net/url"
"strconv"
"strings"
"time"
@@ -319,15 +318,13 @@ func (c S3ApiController) UploadPartCopy(ctx *fiber.Ctx) (*Response, error) {
IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
cs := copySource
copySource, err := url.QueryUnescape(copySource)
err := utils.ValidateCopySource(copySource)
if err != nil {
debuglogger.Logf("error unescaping copy source %q: %v", cs, err)
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, s3err.GetAPIError(s3err.ErrInvalidCopySource)
}, err
}
err = auth.VerifyObjectCopyAccess(ctx.Context(), c.be, copySource,
@@ -456,15 +453,13 @@ func (c S3ApiController) CopyObject(ctx *fiber.Ctx) (*Response, error) {
isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool)
parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
cs := copySource
copySource, err := url.QueryUnescape(copySource)
err := utils.ValidateCopySource(copySource)
if err != nil {
debuglogger.Logf("error unescaping copy source %q: %v", cs, err)
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, s3err.GetAPIError(s3err.ErrInvalidCopySource)
}, err
}
err = auth.VerifyObjectCopyAccess(ctx.Context(), c.be, copySource,
@@ -490,11 +485,12 @@ func (c S3ApiController) CopyObject(ctx *fiber.Ctx) (*Response, error) {
tm, err := time.Parse(iso8601Format, copySrcModifSince)
if err != nil {
debuglogger.Logf("error parsing copy source modified since %q: %v", copySrcModifSince, err)
// TODO: check the error type for invalid values
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, s3err.GetAPIError(s3err.ErrInvalidCopySource)
}, s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)
}
mtime = &tm
}
@@ -503,11 +499,12 @@ func (c S3ApiController) CopyObject(ctx *fiber.Ctx) (*Response, error) {
tm, err := time.Parse(iso8601Format, copySrcUnmodifSince)
if err != nil {
debuglogger.Logf("error parsing copy source unmodified since %q: %v", copySrcUnmodifSince, err)
// TODO: check the error type for invalid values
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, s3err.GetAPIError(s3err.ErrInvalidCopySource)
}, s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)
}
umtime = &tm
}
+10 -7
View File
@@ -585,6 +585,9 @@ func TestS3ApiController_UploadPartCopy(t *testing.T) {
name: "verify access fails",
input: testInput{
locals: accessDeniedLocals,
headers: map[string]string{
"X-Amz-Copy-Source": "bucket/key",
},
},
output: testOutput{
response: &Response{
@@ -612,7 +615,7 @@ func TestS3ApiController_UploadPartCopy(t *testing.T) {
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidCopySource),
err: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
},
{
@@ -806,6 +809,9 @@ func TestS3ApiController_CopyObject(t *testing.T) {
name: "verify access fails",
input: testInput{
locals: accessDeniedLocals,
headers: map[string]string{
"X-Amz-Copy-Source": "bucket/object",
},
},
output: testOutput{
response: &Response{
@@ -820,9 +826,6 @@ func TestS3ApiController_CopyObject(t *testing.T) {
name: "invalid copy source",
input: testInput{
locals: defaultLocals,
headers: map[string]string{
"X-Amz-Copy-Source": "bad%G1",
},
},
output: testOutput{
response: &Response{
@@ -830,7 +833,7 @@ func TestS3ApiController_CopyObject(t *testing.T) {
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidCopySource),
err: s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket),
},
},
{
@@ -848,7 +851,7 @@ func TestS3ApiController_CopyObject(t *testing.T) {
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidCopySource),
err: s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket),
},
},
{
@@ -866,7 +869,7 @@ func TestS3ApiController_CopyObject(t *testing.T) {
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidCopySource),
err: s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket),
},
},
{
+30
View File
@@ -713,3 +713,33 @@ func GetInt64(n *int64) int64 {
return *n
}
// ValidateCopySource parses and validates the copy-source
func ValidateCopySource(copysource string) error {
var err error
copysource, err = url.QueryUnescape(copysource)
if err != nil {
debuglogger.Logf("invalid copy source encoding: %s", copysource)
return s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)
}
bucket, rest, _ := strings.Cut(copysource, "/")
if !IsValidBucketName(bucket) {
debuglogger.Logf("invalid copy source bucket: %s", bucket)
return s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)
}
// cut till the versionId as it's the only query param
// that is recognized in copy source
object, _, _ := strings.Cut(rest, "?versionId=")
// objects containing '../', '...../' ... are considered valid in AWS
// but for the security purposes these should be considered as invalid
// in the gateway
if !IsObjectNameValid(object) {
debuglogger.Logf("invalid copy source object: %s", object)
return s3err.GetAPIError(s3err.ErrInvalidCopySourceObject)
}
return nil
}
+49
View File
@@ -26,6 +26,7 @@ import (
"github.com/aws/aws-sdk-go-v2/service/s3/types"
"github.com/gofiber/fiber/v2"
"github.com/stretchr/testify/assert"
"github.com/valyala/fasthttp"
"github.com/versity/versitygw/backend"
"github.com/versity/versitygw/s3err"
@@ -898,3 +899,51 @@ func TestParseTagging(t *testing.T) {
})
}
}
func TestValidateCopySource(t *testing.T) {
tests := []struct {
name string
copysource string
err error
}{
// invalid encoding
{"invalid encoding 1", "%", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 2", "%2", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 3", "%G1", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 4", "%1Z", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 5", "%0H", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 6", "%XY", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 7", "%E", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 8", "hello%", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 9", "%%", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 10", "%2Gmore", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 11", "100%%sure", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 12", "%#00", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 13", "%0%0", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
{"invalid encoding 14", "%?versionId=id", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)},
// invalid bucket name
{"invalid bucket name 1", "168.200.1.255/obj/foo", s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)},
{"invalid bucket name 2", "/0000:0db8:85a3:0000:0000:8a2e:0370:7224/smth", s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)},
{"invalid bucket name 3", "", s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)},
{"invalid bucket name 4", "//obj/foo", s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)},
{"invalid bucket name 5", "//obj/foo?versionId=id", s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)},
// invalid object name
{"invalid object name 1", "bucket/../foo", s3err.GetAPIError(s3err.ErrInvalidCopySourceObject)},
{"invalid object name 2", "bucket/", s3err.GetAPIError(s3err.ErrInvalidCopySourceObject)},
{"invalid object name 3", "bucket", s3err.GetAPIError(s3err.ErrInvalidCopySourceObject)},
{"invalid object name 4", "bucket/../foo/dir/../../../", s3err.GetAPIError(s3err.ErrInvalidCopySourceObject)},
{"invalid object name 5", "bucket/.?versionId=smth", s3err.GetAPIError(s3err.ErrInvalidCopySourceObject)},
// success
{"no error 1", "bucket/object", nil},
{"no error 2", "bucket/object/key", nil},
{"no error 3", "bucket/4*&(*&(89765))", nil},
{"no error 4", "bucket/foo/../bar", nil},
{"no error 5", "bucket/foo/bar/baz?versionId=id", nil},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidateCopySource(tt.copysource)
assert.Equal(t, tt.err, err)
})
}
}
+18 -6
View File
@@ -87,8 +87,10 @@ const (
ErrInvalidCompleteMpPartNumber
ErrInternalError
ErrInvalidCopyDest
ErrInvalidCopySource
ErrInvalidCopySourceRange
ErrInvalidCopySourceBucket
ErrInvalidCopySourceObject
ErrInvalidCopySourceEncoding
ErrInvalidTagKey
ErrInvalidTagValue
ErrDuplicateTagKey
@@ -333,16 +335,26 @@ var errorCodeResponse = map[ErrorCode]APIError{
Description: "This copy request is illegal because it is trying to copy an object to itself without changing the object's metadata, storage class, website redirect location or encryption attributes.",
HTTPStatusCode: http.StatusBadRequest,
},
ErrInvalidCopySource: {
Code: "InvalidArgument",
Description: "Copy Source must mention the source bucket and key: sourcebucket/sourcekey.",
HTTPStatusCode: http.StatusBadRequest,
},
ErrInvalidCopySourceRange: {
Code: "InvalidArgument",
Description: "The x-amz-copy-source-range value must be of the form bytes=first-last where first and last are the zero-based offsets of the first and last bytes to copy",
HTTPStatusCode: http.StatusBadRequest,
},
ErrInvalidCopySourceBucket: {
Code: "InvalidArgument",
Description: "Invalid copy source bucket name",
HTTPStatusCode: http.StatusBadRequest,
},
ErrInvalidCopySourceObject: {
Code: "InvalidArgument",
Description: "Invalid copy source object key",
HTTPStatusCode: http.StatusBadRequest,
},
ErrInvalidCopySourceEncoding: {
Code: "InvalidArgument",
Description: "Invalid copy source encoding",
HTTPStatusCode: http.StatusBadRequest,
},
ErrInvalidTagKey: {
Code: "InvalidTag",
Description: "The TagKey you have provided is invalid",
+4 -2
View File
@@ -278,7 +278,8 @@ func TestCopyObject(s *S3Conf) {
CopyObject_should_copy_tagging(s)
CopyObject_invalid_tagging_directive(s)
CopyObject_to_itself_with_new_metadata(s)
CopyObject_CopySource_starting_with_slash(s)
CopyObject_copy_source_starting_with_slash(s)
CopyObject_invalid_copy_source(s)
CopyObject_non_existing_dir_object(s)
CopyObject_should_copy_meta_props(s)
CopyObject_should_replace_meta_props(s)
@@ -1062,7 +1063,8 @@ func GetIntTests() IntTests {
"CopyObject_should_copy_tagging": CopyObject_should_copy_tagging,
"CopyObject_invalid_tagging_directive": CopyObject_invalid_tagging_directive,
"CopyObject_to_itself_with_new_metadata": CopyObject_to_itself_with_new_metadata,
"CopyObject_CopySource_starting_with_slash": CopyObject_CopySource_starting_with_slash,
"CopyObject_copy_source_starting_with_slash": CopyObject_copy_source_starting_with_slash,
"CopyObject_invalid_copy_source": CopyObject_invalid_copy_source,
"CopyObject_non_existing_dir_object": CopyObject_non_existing_dir_object,
"CopyObject_should_copy_meta_props": CopyObject_should_copy_meta_props,
"CopyObject_should_replace_meta_props": CopyObject_should_replace_meta_props,
+203 -20
View File
@@ -6617,7 +6617,7 @@ func CopyObject_to_itself_with_new_metadata(s *S3Conf) error {
})
}
func CopyObject_CopySource_starting_with_slash(s *S3Conf) error {
func CopyObject_copy_source_starting_with_slash(s *S3Conf) error {
testName := "CopyObject_CopySource_starting_with_slash"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
dataLength, obj := int64(1234567), "src-obj"
@@ -6681,6 +6681,110 @@ func CopyObject_CopySource_starting_with_slash(s *S3Conf) error {
})
}
func CopyObject_invalid_copy_source(s *S3Conf) error {
testName := "CopyObject_invalid_copy_source"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
for _, test := range []struct {
copySource string
expectedErr s3err.APIError
}{
// invalid encoding
{
// Invalid hex digits
copySource: "bucket/%ZZ",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
{
// Ends with incomplete escape
copySource: "100%/foo/bar/baz",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
{
// Only one digit after %
copySource: "bucket/%A/bar",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
{
// 'G' is not a hex digit
copySource: "bucket/%G1/",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
{
// Just a single percent sign
copySource: "%",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
{
// Only one hex digit
copySource: "bucket/%1",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
{
// Incomplete multibyte UTF-8
copySource: "bucket/%C3%",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
// invalid bucket name
{
// ip v4 address
copySource: "192.168.1.1/foo",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket),
},
{
// ip v6 address
copySource: "2001:0db8:85a3:0000:0000:8a2e:0370:7334/something",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket),
},
{
// some special chars
copySource: "my-buc@k&()t/obj",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket),
},
// invalid object key
{
// object is missing
copySource: "bucket",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceObject),
},
{
// object is missing
copySource: "bucket/",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceObject),
},
// directory navigation object keys
{
copySource: "bucket/.",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceObject),
},
{
copySource: "bucket/..",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceObject),
},
{
copySource: "bucket/../",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceObject),
},
{
copySource: "bucket/foo/ba/../../../r/baz",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceObject),
},
} {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.CopyObject(ctx, &s3.CopyObjectInput{
Bucket: &bucket,
Key: getPtr("obj"),
CopySource: &test.copySource,
})
cancel()
if err := checkApiErr(err, test.expectedErr); err != nil {
return err
}
}
return nil
})
}
func CopyObject_non_existing_dir_object(s *S3Conf) error {
testName := "CopyObject_non_existing_dir_object"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -8762,7 +8866,7 @@ func UploadPartCopy_invalid_part_number(s *S3Conf) error {
partNumber := int32(-10)
_, err := s3client.UploadPartCopy(ctx, &s3.UploadPartCopyInput{
Bucket: &bucket,
CopySource: getPtr("Copy-Source"),
CopySource: getPtr("bucket/key"),
UploadId: getPtr("uploadId"),
Key: getPtr("non-existing-object-key"),
PartNumber: &partNumber,
@@ -8779,25 +8883,104 @@ func UploadPartCopy_invalid_part_number(s *S3Conf) error {
func UploadPartCopy_invalid_copy_source(s *S3Conf) error {
testName := "UploadPartCopy_invalid_copy_source"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
out, err := createMp(s3client, bucket, obj)
if err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
partNumber := int32(1)
_, err = s3client.UploadPartCopy(ctx, &s3.UploadPartCopyInput{
Bucket: &bucket,
CopySource: getPtr("invalid-copy-source"),
UploadId: out.UploadId,
Key: &obj,
PartNumber: &partNumber,
})
cancel()
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidCopySource)); err != nil {
return err
for _, test := range []struct {
copySource string
expectedErr s3err.APIError
}{
// invalid encoding
{
// Invalid hex digits
copySource: "bucket/%ZZ",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
{
// Ends with incomplete escape
copySource: "100%/foo/bar/baz",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
{
// Only one digit after %
copySource: "bucket/%A/bar",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
{
// 'G' is not a hex digit
copySource: "bucket/%G1/",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
{
// Just a single percent sign
copySource: "%",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
{
// Only one hex digit
copySource: "bucket/%1",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
{
// Incomplete multibyte UTF-8
copySource: "bucket/%C3%",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding),
},
// invalid bucket name
{
// ip v4 address
copySource: "192.168.1.1/foo",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket),
},
{
// ip v6 address
copySource: "2001:0db8:85a3:0000:0000:8a2e:0370:7334/something",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket),
},
{
// some special chars
copySource: "my-buc@k&()t/obj",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket),
},
// invalid object key
{
// object is missing
copySource: "bucket",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceObject),
},
{
// object is missing
copySource: "bucket/",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceObject),
},
// directory navigation object keys
{
copySource: "bucket/.",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceObject),
},
{
copySource: "bucket/..",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceObject),
},
{
copySource: "bucket/../",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceObject),
},
{
copySource: "bucket/foo/ba/../../../r/baz",
expectedErr: s3err.GetAPIError(s3err.ErrInvalidCopySourceObject),
},
} {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.UploadPartCopy(ctx, &s3.UploadPartCopyInput{
Bucket: &bucket,
Key: getPtr("obj"),
UploadId: getPtr("mock-upload-id"),
CopySource: &test.copySource,
PartNumber: &partNumber,
})
cancel()
if err := checkApiErr(err, test.expectedErr); err != nil {
return err
}
}
return nil