mirror of
https://github.com/versity/versitygw.git
synced 2026-05-13 07:21:28 +00:00
fix: forward slash url encoded used as bucket/key separator
handle %2F ("/") url encoded separator for bucket/key
Fixes #2024
---------
Co-authored-by: Galie Alexandru MTANA <alexandru.galie@mt.com>
This commit is contained in:
@@ -18,6 +18,7 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/url"
|
||||
"strings"
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3"
|
||||
@@ -37,8 +38,15 @@ func VerifyObjectCopyAccess(ctx context.Context, be backend.Backend, copySource
|
||||
if err := VerifyAccess(ctx, be, opts); err != nil {
|
||||
return err
|
||||
}
|
||||
// Verify source bucket access
|
||||
srcBucket, srcObject, found := strings.Cut(copySource, "/")
|
||||
// Verify source bucket access.
|
||||
// URL-decode the copy source before splitting so that clients which send
|
||||
// the bucket/key separator as "%2F" are handled correctly.
|
||||
// Callers are expected to have already stripped any leading '/'.
|
||||
decodedSrc, err := url.QueryUnescape(copySource)
|
||||
if err != nil {
|
||||
return s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)
|
||||
}
|
||||
srcBucket, srcObject, found := strings.Cut(decodedSrc, "/")
|
||||
if !found {
|
||||
return s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)
|
||||
}
|
||||
|
||||
129
auth/access-control_test.go
Normal file
129
auth/access-control_test.go
Normal file
@@ -0,0 +1,129 @@
|
||||
// Copyright 2026 Versity Software
|
||||
// This file is licensed under the Apache License, Version 2.0
|
||||
// (the "License"); you may not use this file except in compliance
|
||||
// with the License. You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing,
|
||||
// software distributed under the License is distributed on an
|
||||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
// KIND, either express or implied. See the License for the
|
||||
// specific language governing permissions and limitations
|
||||
// under the License.
|
||||
|
||||
package auth
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3"
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/versity/versitygw/backend"
|
||||
"github.com/versity/versitygw/s3err"
|
||||
)
|
||||
|
||||
// noBucketPolicyBackend is a test stub that returns ErrNoSuchBucketPolicy for
|
||||
// GetBucketPolicy and serves a configurable ACL for GetBucketAcl.
|
||||
type noBucketPolicyBackend struct {
|
||||
backend.BackendUnsupported
|
||||
srcAcl ACL
|
||||
}
|
||||
|
||||
func (b noBucketPolicyBackend) GetBucketPolicy(_ context.Context, _ string) ([]byte, error) {
|
||||
return nil, s3err.GetAPIError(s3err.ErrNoSuchBucketPolicy)
|
||||
}
|
||||
|
||||
func (b noBucketPolicyBackend) GetBucketAcl(_ context.Context, _ *s3.GetBucketAclInput) ([]byte, error) {
|
||||
return json.Marshal(b.srcAcl)
|
||||
}
|
||||
|
||||
func TestVerifyObjectCopyAccess_URLEncodedSlashSeparator(t *testing.T) {
|
||||
const testUser = "testuser"
|
||||
|
||||
// Source bucket ACL: grants READ to testUser.
|
||||
srcAcl := ACL{
|
||||
Owner: "owner",
|
||||
Grantees: []Grantee{
|
||||
{
|
||||
Access: testUser,
|
||||
Permission: PermissionRead,
|
||||
Type: types.TypeCanonicalUser,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
be := noBucketPolicyBackend{srcAcl: srcAcl}
|
||||
|
||||
// Destination bucket ACL: testUser is the owner (DisableACL=true path).
|
||||
opts := AccessOptions{
|
||||
Acl: ACL{Owner: testUser},
|
||||
AclPermission: PermissionWrite,
|
||||
IsRoot: false,
|
||||
Acc: Account{Access: testUser, Role: RoleUser},
|
||||
Bucket: "dst-bucket",
|
||||
Object: "dst-key",
|
||||
Actions: []Action{PutObjectAction},
|
||||
DisableACL: true,
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
copySource string
|
||||
}{
|
||||
{
|
||||
name: "percent-encoded slash (%2F) as bucket/key separator",
|
||||
copySource: "my-namespace-test-container%2Ftest-blob",
|
||||
},
|
||||
{
|
||||
name: "%2F separator with encoded chars in key",
|
||||
copySource: "src-bucket%2Fmy%20folder%2Fmy-key",
|
||||
},
|
||||
{
|
||||
name: "%2F separator with versionId",
|
||||
copySource: "src-bucket%2Fsrc-key?versionId=abc123",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
err := VerifyObjectCopyAccess(context.Background(), be, tt.copySource, opts)
|
||||
assert.NoError(t, err,
|
||||
"should accept %%2F as the bucket/key separator in x-amz-copy-source")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestVerifyObjectCopyAccess_LiteralSlashSeparator(t *testing.T) {
|
||||
const testUser = "testuser"
|
||||
|
||||
srcAcl := ACL{
|
||||
Owner: "owner",
|
||||
Grantees: []Grantee{
|
||||
{
|
||||
Access: testUser,
|
||||
Permission: PermissionRead,
|
||||
Type: types.TypeCanonicalUser,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
be := noBucketPolicyBackend{srcAcl: srcAcl}
|
||||
|
||||
opts := AccessOptions{
|
||||
Acl: ACL{Owner: testUser},
|
||||
AclPermission: PermissionWrite,
|
||||
IsRoot: false,
|
||||
Acc: Account{Access: testUser, Role: RoleUser},
|
||||
Bucket: "dst-bucket",
|
||||
Object: "dst-key",
|
||||
Actions: []Action{PutObjectAction},
|
||||
DisableACL: true,
|
||||
}
|
||||
|
||||
err := VerifyObjectCopyAccess(context.Background(), be, "src-bucket/src-key", opts)
|
||||
assert.NoError(t, err, "literal slash separator should work")
|
||||
}
|
||||
@@ -232,29 +232,28 @@ func ParseCopySource(copySourceHeader string) (string, string, string, error) {
|
||||
copySourceHeader = copySourceHeader[1:]
|
||||
}
|
||||
|
||||
var copySource, versionId string
|
||||
// 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 {
|
||||
copySource = copySourceHeader
|
||||
rawSource = copySourceHeader
|
||||
} else {
|
||||
copySource = copySourceHeader[:i]
|
||||
rawSource = copySourceHeader[:i]
|
||||
versionId = copySourceHeader[i+11:]
|
||||
}
|
||||
|
||||
srcBucket, srcObject, ok := strings.Cut(copySource, "/")
|
||||
if !ok {
|
||||
return "", "", "", s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)
|
||||
// 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)
|
||||
if err != nil {
|
||||
return "", "", "", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)
|
||||
}
|
||||
|
||||
var err error
|
||||
// URL-decode the bucket and object names to handle special characters
|
||||
srcBucket, err = url.QueryUnescape(srcBucket)
|
||||
if err != nil {
|
||||
return "", "", "", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)
|
||||
}
|
||||
srcObject, err = url.QueryUnescape(srcObject)
|
||||
if err != nil {
|
||||
return "", "", "", s3err.GetAPIError(s3err.ErrInvalidCopySourceEncoding)
|
||||
srcBucket, srcObject, ok := strings.Cut(decoded, "/")
|
||||
if !ok {
|
||||
return "", "", "", s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket)
|
||||
}
|
||||
|
||||
return srcBucket, srcObject, versionId, nil
|
||||
|
||||
@@ -87,6 +87,38 @@ func TestParseCopySource(t *testing.T) {
|
||||
wantVersionId: "xyz789",
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "percent-encoded slash as bucket/key separator",
|
||||
copySourceHeader: "my-namespace-test-container%2Ftest-blob",
|
||||
wantBucket: "my-namespace-test-container",
|
||||
wantObject: "test-blob",
|
||||
wantVersionId: "",
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "percent-encoded slash separator with leading slash",
|
||||
copySourceHeader: "/my-namespace-test-container%2Ftest-blob",
|
||||
wantBucket: "my-namespace-test-container",
|
||||
wantObject: "test-blob",
|
||||
wantVersionId: "",
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "percent-encoded slash separator with versionId",
|
||||
copySourceHeader: "my-bucket%2Fmy-object?versionId=abc123",
|
||||
wantBucket: "my-bucket",
|
||||
wantObject: "my-object",
|
||||
wantVersionId: "abc123",
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "percent-encoded slash separator with encoded object key",
|
||||
copySourceHeader: "my-bucket%2Fmy%20folder%2Fmy%20object",
|
||||
wantBucket: "my-bucket",
|
||||
wantObject: "my folder/my object",
|
||||
wantVersionId: "",
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "invalid URL encoding - incomplete escape",
|
||||
copySourceHeader: "mybucket/object%",
|
||||
|
||||
Reference in New Issue
Block a user