feat: adds a middleware to validate bucket/object names

Implements a middleware that validates incoming bucket and object names before authentication. This helps prevent malicious attacks that attempt to access restricted or unreachable data in `POSIX`.

Adds test cases to cover such attack scenarios, including false negatives where encoded paths are used to try accessing resources outside the intended bucket.

Removes bucket validation from all other layers—including `controllers` and both `POSIX` and `ScoutFS` backends — by moving the logic entirely into the middleware layer.

backend/posix/posix.go

@@ -334,10 +334,6 @@ func (p *Posix) ListBuckets(_ context.Context, input s3response.ListBucketsInput
 }
 
 func (p *Posix) HeadBucket(_ context.Context, input *s3.HeadBucketInput) (*s3.HeadBucketOutput, error) {
-	if input.Bucket == nil {
-		return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
-
 	_, err := os.Lstat(*input.Bucket)
 	if errors.Is(err, fs.ErrNotExist) {
 		return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket)
@@ -350,10 +346,6 @@ func (p *Posix) HeadBucket(_ context.Context, input *s3.HeadBucketInput) (*s3.He
 }
 
 func (p *Posix) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, acl []byte) error {
-	if input.Bucket == nil {
-		return s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
-
 	acct, ok := ctx.Value("account").(auth.Account)
 	if !ok {
 		acct = auth.Account{}
@@ -465,10 +457,6 @@ func (p *Posix) isBucketEmpty(bucket string) error {
 }
 
 func (p *Posix) DeleteBucket(_ context.Context, bucket string) error {
-	if bucket == "" {
-		return s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
-
 	// Check if the bucket is empty
 	err := p.isBucketEmpty(bucket)
 	if err != nil {
@@ -1189,9 +1177,6 @@ func (p *Posix) fileToObjVersions(bucket string) backend.GetVersionsFunc {
 }
 
 func (p *Posix) CreateMultipartUpload(ctx context.Context, mpu s3response.CreateMultipartUploadInput) (s3response.InitiateMultipartUploadResult, error) {
-	if mpu.Bucket == nil {
-		return s3response.InitiateMultipartUploadResult{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	if mpu.Key == nil {
 		return s3response.InitiateMultipartUploadResult{}, s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
@@ -1380,9 +1365,6 @@ func (p *Posix) CompleteMultipartUpload(ctx context.Context, input *s3.CompleteM
 
 	var res s3response.CompleteMultipartUploadResult
 
-	if input.Bucket == nil {
-		return res, "", s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	if input.Key == nil {
 		return res, "", s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
@@ -1950,9 +1932,6 @@ func isValidMeta(val string) bool {
 }
 
 func (p *Posix) AbortMultipartUpload(_ context.Context, mpu *s3.AbortMultipartUploadInput) error {
-	if mpu.Bucket == nil {
-		return s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	if mpu.Key == nil {
 		return s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
@@ -1992,10 +1971,6 @@ func (p *Posix) AbortMultipartUpload(_ context.Context, mpu *s3.AbortMultipartUp
 func (p *Posix) ListMultipartUploads(_ context.Context, mpu *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResult, error) {
 	var lmu s3response.ListMultipartUploadsResult
 
-	if mpu.Bucket == nil {
-		return lmu, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
-
 	bucket := *mpu.Bucket
 	var delimiter string
 	if mpu.Delimiter != nil {
@@ -2143,9 +2118,6 @@ func (p *Posix) ListMultipartUploads(_ context.Context, mpu *s3.ListMultipartUpl
 func (p *Posix) ListParts(ctx context.Context, input *s3.ListPartsInput) (s3response.ListPartsResult, error) {
 	var lpr s3response.ListPartsResult
 
-	if input.Bucket == nil {
-		return lpr, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	if input.Key == nil {
 		return lpr, s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
@@ -2299,9 +2271,6 @@ func (p *Posix) UploadPart(ctx context.Context, input *s3.UploadPartInput) (*s3.
 		acct = auth.Account{}
 	}
 
-	if input.Bucket == nil {
-		return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	if input.Key == nil {
 		return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
@@ -2469,9 +2438,6 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput)
 		acct = auth.Account{}
 	}
 
-	if upi.Bucket == nil {
-		return s3response.CopyPartResult{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	if upi.Key == nil {
 		return s3response.CopyPartResult{}, s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
@@ -2683,9 +2649,6 @@ func (p *Posix) PutObject(ctx context.Context, po s3response.PutObjectInput) (s3
 		acct = auth.Account{}
 	}
 
-	if po.Bucket == nil {
-		return s3response.PutObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	if po.Key == nil {
 		return s3response.PutObjectOutput{}, s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
@@ -3003,9 +2966,6 @@ func (p *Posix) PutObject(ctx context.Context, po s3response.PutObjectInput) (s3
 }
 
 func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (*s3.DeleteObjectOutput, error) {
-	if input.Bucket == nil {
-		return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	if input.Key == nil {
 		return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
@@ -3361,9 +3321,6 @@ func (p *Posix) DeleteObjects(ctx context.Context, input *s3.DeleteObjectsInput)
 }
 
 func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetObjectOutput, error) {
-	if input.Bucket == nil {
-		return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	if input.Key == nil {
 		return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
@@ -3592,9 +3549,6 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
 }
 
 func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3.HeadObjectOutput, error) {
-	if input.Bucket == nil {
-		return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	if input.Key == nil {
 		return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
@@ -3862,9 +3816,6 @@ func (p *Posix) GetObjectAttributes(ctx context.Context, input *s3.GetObjectAttr
 }
 
 func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput) (s3response.CopyObjectOutput, error) {
-	if input.Bucket == nil {
-		return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	if input.Key == nil {
 		return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidCopyDest)
 	}
@@ -4177,9 +4128,6 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput
 }
 
 func (p *Posix) ListObjects(ctx context.Context, input *s3.ListObjectsInput) (s3response.ListObjectsResult, error) {
-	if input.Bucket == nil {
-		return s3response.ListObjectsResult{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	bucket := *input.Bucket
 	prefix := ""
 	if input.Prefix != nil {
@@ -4329,9 +4277,6 @@ func (p *Posix) fileToObj(bucket string, fetchOwner bool) backend.GetObjFunc {
 }
 
 func (p *Posix) ListObjectsV2(ctx context.Context, input *s3.ListObjectsV2Input) (s3response.ListObjectsV2Result, error) {
-	if input.Bucket == nil {
-		return s3response.ListObjectsV2Result{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	bucket := *input.Bucket
 	prefix := ""
 	if input.Prefix != nil {
@@ -4408,9 +4353,6 @@ func (p *Posix) PutBucketAcl(_ context.Context, bucket string, data []byte) erro
 }
 
 func (p *Posix) GetBucketAcl(_ context.Context, input *s3.GetBucketAclInput) ([]byte, error) {
-	if input.Bucket == nil {
-		return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	_, err := os.Stat(*input.Bucket)
 	if errors.Is(err, fs.ErrNotExist) {
 		return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket)

backend/scoutfs/scoutfs.go

@@ -201,9 +201,6 @@ func (s *ScoutFS) CompleteMultipartUpload(ctx context.Context, input *s3.Complet
 
 	var res s3response.CompleteMultipartUploadResult
 
-	if input.Bucket == nil {
-		return res, "", s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	if input.Key == nil {
 		return res, "", s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
@@ -730,9 +727,6 @@ func (s *ScoutFS) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.
 }
 
 func (s *ScoutFS) ListObjects(ctx context.Context, input *s3.ListObjectsInput) (s3response.ListObjectsResult, error) {
-	if input.Bucket == nil {
-		return s3response.ListObjectsResult{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	bucket := *input.Bucket
 	prefix := ""
 	if input.Prefix != nil {
@@ -780,9 +774,6 @@ func (s *ScoutFS) ListObjects(ctx context.Context, input *s3.ListObjectsInput) (
 }
 
 func (s *ScoutFS) ListObjectsV2(ctx context.Context, input *s3.ListObjectsV2Input) (s3response.ListObjectsV2Result, error) {
-	if input.Bucket == nil {
-		return s3response.ListObjectsV2Result{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
-	}
 	bucket := *input.Bucket
 	prefix := ""
 	if input.Prefix != nil {

s3api/controllers/base.go

@@ -1720,14 +1720,6 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error {
 			})
 	}
 
-	if ok := utils.IsValidBucketName(bucket); !ok {
-		return SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidBucketName),
-			&MetaOpts{
-				Logger:     c.logger,
-				MetricsMng: c.mm,
-				Action:     metrics.ActionCreateBucket,
-			})
-	}
 	if ok := utils.IsValidOwnership(objectOwnership); !ok {
 		return SendResponse(ctx, s3err.APIError{
 			Code:           "InvalidArgument",

s3api/controllers/base_test.go

@@ -884,15 +884,6 @@ func TestS3ApiController_PutBucketActions(t *testing.T) {
 			wantErr:    false,
 			statusCode: 400,
 		},
-		{
-			name: "Create-bucket-invalid-bucket-name",
-			app:  app,
-			args: args{
-				req: httptest.NewRequest(http.MethodPut, "/aa", nil),
-			},
-			wantErr:    false,
-			statusCode: 400,
-		},
 		{
 			name: "Create-bucket-success",
 			app:  app,

s3api/middlewares/bucket-object-name-validator.go

@@ -0,0 +1,58 @@
+// Copyright 2023 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 middlewares
+
+import (
+	"net/http"
+
+	"github.com/gofiber/fiber/v2"
+	"github.com/versity/versitygw/metrics"
+	"github.com/versity/versitygw/s3api/utils"
+	"github.com/versity/versitygw/s3err"
+	"github.com/versity/versitygw/s3log"
+)
+
+// BucketObjectNameValidator extracts and validates
+// the bucket and object names from the request URI.
+func BucketObjectNameValidator(l s3log.AuditLogger, mm *metrics.Manager) fiber.Handler {
+	return func(ctx *fiber.Ctx) error {
+		// skip the check for admin apis
+		if ctx.Method() == http.MethodPatch {
+			return ctx.Next()
+		}
+
+		path := ctx.Path()
+		// skip the check if the operation isn't bucket/object scoped
+		// e.g ListBuckets
+		if path == "/" {
+			return ctx.Next()
+		}
+
+		bucket, object := parsePath(path)
+
+		// check if the provided bucket name is valid
+		if !utils.IsValidBucketName(bucket) {
+			return sendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidBucketName), l, mm)
+		}
+
+		// check if the provided object name is valid
+		// skip for empty objects: e.g bucket operations: HeadBucket...
+		if object != "" && !utils.IsObjectNameValid(object) {
+			return sendResponse(ctx, s3err.GetAPIError(s3err.ErrBadRequest), l, mm)
+		}
+
+		return ctx.Next()
+	}
+}

s3api/server.go

@@ -91,6 +91,9 @@ func New(
 		app.Use(middlewares.DebugLogger())
 	}
 
+	// initialize the bucket/object name validator
+	app.Use(middlewares.BucketObjectNameValidator(l, mm))
+
 	// Public buckets access checker
 	app.Use(middlewares.AuthorizePublicBucketAccess(be, l, mm))
 

s3err/s3err.go

@@ -167,6 +167,7 @@ const (
 	ErrChecksumTypeWithAlgo
 	ErrInvalidChecksumHeader
 	ErrTrailerHeaderNotSupported
+	ErrBadRequest
 
 	// Non-AWS errors
 	ErrExistingObjectIsDirectory
@@ -726,6 +727,11 @@ var errorCodeResponse = map[ErrorCode]APIError{
 		Description:    "The value specified in the x-amz-trailer header is not supported",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
+	ErrBadRequest: {
+		Code:           "400",
+		Description:    "Bad Request",
+		HTTPStatusCode: http.StatusBadRequest,
+	},
 
 	// non aws errors
 	ErrExistingObjectIsDirectory: {

tests/integration/group-tests.go

@@ -150,12 +150,15 @@ func TestPutObject(s *S3Conf) {
 		PutObject_incorrect_checksums(s)
 		PutObject_default_checksum(s)
 		PutObject_checksums_success(s)
+		// azure applies some encoding mechanisms.
+		PutObject_false_negative_object_names(s)
 	}
 	PutObject_success(s)
 	if !s.versioningEnabled {
 		PutObject_racey_success(s)
 	}
 	PutObject_invalid_credentials(s)
+	PutObject_invalid_object_names(s)
 }
 
 func TestHeadObject(s *S3Conf) {
@@ -969,6 +972,8 @@ func GetIntTests() IntTests {
 		"PutObject_special_chars":                                                 PutObject_special_chars,
 		"PutObject_tagging":                                                       PutObject_tagging,
 		"PutObject_success":                                                       PutObject_success,
+		"PutObject_invalid_object_names":                                          PutObject_invalid_object_names,
+		"PutObject_false_negative_object_names":                                   PutObject_false_negative_object_names,
 		"PutObject_racey_success":                                                 PutObject_racey_success,
 		"HeadObject_non_existing_object":                                          HeadObject_non_existing_object,
 		"HeadObject_invalid_part_number":                                          HeadObject_invalid_part_number,

tests/integration/tests.go

@@ -3415,6 +3415,81 @@ func PutObject_invalid_credentials(s *S3Conf) error {
 	})
 }
 
+func PutObject_invalid_object_names(s *S3Conf) error {
+	testName := "PutObject_invalid_object_names"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		for _, obj := range []string{
+			".",
+			"..",
+			"./",
+			"/.",
+			"//",
+			"../",
+			"/..",
+			"/..",
+			"../.",
+			"../../../.",
+			"../../../etc/passwd",
+			"../../../../tmp/foo",
+			"for/../../bar/",
+			"a/a/a/../../../../../etc/passwd",
+			"/a/../../b/../../c/../../../etc/passwd",
+		} {
+			_, err := putObjects(s3client, []string{obj}, bucket)
+			if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrBadRequest)); err != nil {
+				return err
+			}
+		}
+
+		return nil
+	})
+}
+
+func PutObject_false_negative_object_names(s *S3Conf) error {
+	testName := "PutObject_false_negative_object_names"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		objs := []string{
+			"%252e%252e%252fetc/passwd",            // double encoding
+			"%2e%2e/%2e%2e/%2e%2e/.ssh/id_rsa",     // double URL-encoded
+			"%u002e%u002e/%u002e%u002e/etc/passwd", // unicode escape
+			"..%2f..%2f..%2fsecret/file.txt",       // URL-encoded
+			"..%c0%af..%c0%afetc/passwd",           // UTF-8 overlong trick
+			".../.../.../target.txt",
+			"..\\u2215..\\u2215etc/passwd",             // Unicode division slash
+			"dir/%20../file.txt",                       // encoded space
+			"dir/%c0%ae%c0%ae/%c0%ae%c0%ae/etc/passwd", // overlong UTF-8 encoding
+			"logs/latest -> /etc/passwd",               // symlink attacks
+			//TODO: add this test case in advanced routing
+			// "/etc/passwd" // absolute path injection
+		}
+		_, err := putObjects(s3client, objs, bucket)
+		if err != nil {
+			return err
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		res, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{
+			Bucket: &bucket,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		if len(res.Contents) != len(objs) {
+			return fmt.Errorf("expected %v objects, instead got %v", len(objs), len(res.Contents))
+		}
+
+		for i, obj := range res.Contents {
+			if *obj.Key != objs[i] {
+				return fmt.Errorf("expected the %vth object name to be %s, instead got %s", i+1, objs[i], *obj.Key)
+			}
+		}
+
+		return nil
+	})
+}
+
 func HeadObject_non_existing_object(s *S3Conf) error {
 	testName := "HeadObject_non_existing_object"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -9995,7 +10070,7 @@ func AbortMultipartUpload_non_existing_bucket(s *S3Conf) error {
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
 		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
 		_, err := s3client.AbortMultipartUpload(ctx, &s3.AbortMultipartUploadInput{
-			Bucket:   getPtr("incorrectBucket"),
+			Bucket:   getPtr("incorrect-bucket"),
 			Key:      getPtr("my-obj"),
 			UploadId: getPtr("uploadId"),
 		})
@@ -12328,7 +12403,7 @@ func PutBucketPolicy_non_existing_bucket(s *S3Conf) error {
 		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
 		doc := genPolicyDoc("Allow", `"*"`, `"s3:*"`, fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket))
 		_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
-			Bucket: getPtr("non_existing_bucket"),
+			Bucket: getPtr("non-existing-bucket"),
 			Policy: &doc,
 		})
 		cancel()
@@ -13003,7 +13078,7 @@ func GetBucketPolicy_non_existing_bucket(s *S3Conf) error {
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
 		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
 		_, err := s3client.GetBucketPolicy(ctx, &s3.GetBucketPolicyInput{
-			Bucket: getPtr("non_existing_bucket"),
+			Bucket: getPtr("non-existing-bucket"),
 		})
 		cancel()
 
@@ -13069,7 +13144,7 @@ func DeleteBucketPolicy_non_existing_bucket(s *S3Conf) error {
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
 		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
 		_, err := s3client.DeleteBucketPolicy(ctx, &s3.DeleteBucketPolicyInput{
-			Bucket: getPtr("non_existing_bucket"),
+			Bucket: getPtr("non-existing-bucket"),
 		})
 		cancel()