fix: Fixes #246, Added max length check for tag keys and values in Pu…

…tObjectTagging and PutObject actions
versity · Sep 19, 2023 · a77954a · a77954a
1 parent c9653cf
commit a77954a
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 0 deletions.
diff --git a/backend/posix/posix.go b/backend/posix/posix.go
@@ -948,6 +948,9 @@ func (p *Posix) PutObject(ctx context.Context, po *s3.PutObjectInput) (string, e
 			if len(p) != 2 {
 				return "", s3err.GetAPIError(s3err.ErrInvalidTag)
 			}
+			if len(p[0]) > 128 || len(p[1]) > 256 {
+				return "", s3err.GetAPIError(s3err.ErrInvalidTag)
+			}
 			tags[p[0]] = p[1]
 		}
 	}

diff --git a/integration/action-tests.go b/integration/action-tests.go
@@ -50,6 +50,7 @@ func TestPutObject(s *S3Conf) {
 	PutObject_special_chars(s)
 	PutObject_existing_dir_obj(s)
 	PutObject_obj_parent_is_file(s)
+	PutObject_invalid_long_tags(s)
 	PutObject_success(s)
 }
 
@@ -95,6 +96,7 @@ func TestCopyObject(s *S3Conf) {
 
 func TestPutObjectTagging(s *S3Conf) {
 	PutObjectTagging_non_existing_object(s)
+	PutObjectTagging_long_tags(s)
 	PutObjectTagging_success(s)
 }
 

diff --git a/integration/tests.go b/integration/tests.go
@@ -1022,6 +1022,42 @@ func PutObject_obj_parent_is_file(s *S3Conf) {
 	})
 }
 
+func PutObject_invalid_long_tags(s *S3Conf) {
+	testName := "PutObject_invalid_long_tags"
+	actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		key := "my-obj"
+		tagging := fmt.Sprintf("%v=val", genRandString(200))
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err := s3client.PutObject(ctx, &s3.PutObjectInput{
+			Bucket:  &bucket,
+			Key:     &key,
+			Tagging: &tagging,
+		})
+		cancel()
+
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidTag)); err != nil {
+			return err
+		}
+
+		tagging = fmt.Sprintf("key=%v", genRandString(300))
+
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.PutObject(ctx, &s3.PutObjectInput{
+			Bucket:  &bucket,
+			Key:     &key,
+			Tagging: &tagging,
+		})
+		cancel()
+
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidTag)); err != nil {
+			return err
+		}
+
+		return nil
+	})
+}
+
 func PutObject_success(s *S3Conf) {
 	testName := "PutObject_success"
 	actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -1716,6 +1752,11 @@ func CopyObject_not_owned_source_bucket(s *S3Conf) {
 		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
 			return err
 		}
+
+		err = teardown(s, dstBucket)
+		if err != nil {
+			return err
+		}
 		return nil
 	})
 }
@@ -1819,6 +1860,42 @@ func PutObjectTagging_non_existing_object(s *S3Conf) {
 	})
 }
 
+func PutObjectTagging_long_tags(s *S3Conf) {
+	testName := "PutObjectTagging_long_tags"
+	actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		obj := "my-obj"
+		tagging := types.Tagging{TagSet: []types.Tag{{Key: getPtr(genRandString(129)), Value: getPtr("val")}}}
+		err := putObjects(s3client, []string{obj}, bucket)
+		if err != nil {
+			return err
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.PutObjectTagging(ctx, &s3.PutObjectTaggingInput{
+			Bucket:  &bucket,
+			Key:     &obj,
+			Tagging: &tagging})
+		cancel()
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidTag)); err != nil {
+			return err
+		}
+
+		tagging = types.Tagging{TagSet: []types.Tag{{Key: getPtr("key"), Value: getPtr(genRandString(257))}}}
+
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.PutObjectTagging(ctx, &s3.PutObjectTaggingInput{
+			Bucket:  &bucket,
+			Key:     &obj,
+			Tagging: &tagging})
+		cancel()
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidTag)); err != nil {
+			return err
+		}
+
+		return nil
+	})
+}
+
 func PutObjectTagging_success(s *S3Conf) {
 	testName := "PutObjectTagging_success"
 	actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -2462,6 +2539,11 @@ func UploadPartCopy_greater_range_than_obj_size(s *S3Conf) {
 			return err
 		}
 
+		err = teardown(s, srcBucket)
+		if err != nil {
+			return err
+		}
+
 		return nil
 	})
 }

diff --git a/integration/utils.go b/integration/utils.go
@@ -10,6 +10,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	rnd "math/rand"
 	"net/http"
 	"os"
 	"os/exec"
@@ -526,3 +527,15 @@ func changeBucketsOwner(s *S3Conf, buckets []string, owner string) error {
 
 	return nil
 }
+
+const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
+
+func genRandString(length int) string {
+	source := rnd.NewSource(time.Now().UnixNano())
+	random := rnd.New(source)
+	result := make([]byte, length)
+	for i := range result {
+		result[i] = charset[random.Intn(len(charset))]
+	}
+	return string(result)
+}
diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go
@@ -446,6 +446,9 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error {
 		tags := make(map[string]string, len(objTagging.TagSet.Tags))
 
 		for _, tag := range objTagging.TagSet.Tags {
+			if len(tag.Key) > 128 || len(tag.Value) > 256 {
+				return SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidTag), &MetaOpts{Logger: c.logger, Action: "PutObjectTagging", BucketOwner: parsedAcl.Owner})
+			}
 			tags[tag.Key] = tag.Value
 		}