mirror of
https://github.com/versity/versitygw.git
synced 2026-09-24 17:04:16 +00:00
fix: adds versionId validation for object level actions
Fixes #1630 S3 returns `InvalidArgument: Invalid version id specified` for invalid version IDs in object-level actions that accept `versionId` as a query parameter. The `versionId` in S3 follows a specific structure, and if the input string doesn’t match this structure, the error is returned. In the gateway, the `versionId` is generated using the `ulid` package, which also has a defined structure. This PR adds validation for object-level operations that work with object versions by using the ULID parser. These actions include: `HeadObject`, `GetObject`, `PutObjectTagging`, `GetObjectTagging`, `DeleteObjectTagging`, `PutObjectLegalHold`, `GetObjectLegalHold`, `PutObjectRetention`, `GetObjectRetention`, `DeleteObject`, `CopyObject`, `UploadPartCopy`, and `GetObjectAttributes`.
This commit is contained in:
+22
-1
@@ -31,6 +31,7 @@ import (
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
||||
"github.com/gofiber/fiber/v2"
|
||||
"github.com/oklog/ulid/v2"
|
||||
"github.com/valyala/fasthttp"
|
||||
"github.com/versity/versitygw/debuglogger"
|
||||
"github.com/versity/versitygw/s3err"
|
||||
@@ -818,7 +819,7 @@ func ValidateCopySource(copysource string) error {
|
||||
|
||||
// cut till the versionId as it's the only query param
|
||||
// that is recognized in copy source
|
||||
object, _, _ := strings.Cut(rest, "?versionId=")
|
||||
object, versionId, _ := strings.Cut(rest, "?versionId=")
|
||||
|
||||
// objects containing '../', '...../' ... are considered valid in AWS
|
||||
// but for the security purposes these should be considered as invalid
|
||||
@@ -828,6 +829,12 @@ func ValidateCopySource(copysource string) error {
|
||||
return s3err.GetAPIError(s3err.ErrInvalidCopySourceObject)
|
||||
}
|
||||
|
||||
// validate the versionId
|
||||
err = ValidateVersionId(versionId)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -847,3 +854,17 @@ func ApplyOverride(original, override *string) *string {
|
||||
}
|
||||
return original
|
||||
}
|
||||
|
||||
// ValidateVersionId check if the input versionId is 'ulid' compatible
|
||||
func ValidateVersionId(versionId string) error {
|
||||
if versionId == "" || versionId == "null" {
|
||||
return nil
|
||||
}
|
||||
_, err := ulid.Parse(versionId)
|
||||
if err != nil {
|
||||
debuglogger.Logf("invalid versionId: %s", versionId)
|
||||
return s3err.GetAPIError(s3err.ErrInvalidVersionId)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -955,12 +955,15 @@ func TestValidateCopySource(t *testing.T) {
|
||||
{"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)},
|
||||
// invalid versionId
|
||||
{"invalid versionId 1", "bucket/object?versionId=invalid", s3err.GetAPIError(s3err.ErrInvalidVersionId)},
|
||||
{"invalid versionId 2", "bucket/object?versionId=01BX5ZZKBKACTAV9WEVGEMMV", s3err.GetAPIError(s3err.ErrInvalidVersionId)},
|
||||
// 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},
|
||||
{"no error 5", "bucket/foo/bar/baz?versionId=01BX5ZZKBKACTAV9WEVGEMMVRZ", nil},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user