fix: reject duplicate keys in PostPolicyJSON document (#11902)

fixes #11894

cmd/bucket-handlers.go

@@ -17,6 +17,7 @@
 package cmd
 
 import (
+	"bytes"
 	"crypto/subtle"
 	"encoding/base64"
 	"encoding/xml"
@@ -926,10 +927,11 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h
 
 	// Handle policy if it is set.
 	if len(policyBytes) > 0 {
-
+		postPolicyForm, err := parsePostPolicyForm(bytes.NewReader(policyBytes))
-		postPolicyForm, err := parsePostPolicyForm(string(policyBytes))
 		if err != nil {
-			writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrPostPolicyConditionInvalidFormat), r.URL, guessIsBrowserReq(r))
+			errAPI := errorCodes.ToAPIErr(ErrPostPolicyConditionInvalidFormat)
+			errAPI.Description = fmt.Sprintf("%s '(%s)'", errAPI.Description, err)
+			writeErrorResponse(ctx, w, errAPI, r.URL, guessIsBrowserReq(r))
 			return
 		}
 

cmd/erasure-sets.go

@@ -431,8 +431,10 @@ func newErasureSets(ctx context.Context, endpoints Endpoints, storageDisks []Sto
 		}
 	}
 
-	// cleanup ".trash/" folder every 10 minutes with sufficient sleep cycles.
+	// cleanup ".trash/" folder every 5m minutes with sufficient sleep cycles, between each
-	deletedObjectsCleanupInterval, err := time.ParseDuration(env.Get(envMinioDeleteCleanupInterval, "10m"))
+	// deletes a dynamic sleeper is used with a factor of 10 ratio with max delay between
+	// deletes to be 2 seconds.
+	deletedObjectsCleanupInterval, err := time.ParseDuration(env.Get(envMinioDeleteCleanupInterval, "5m"))
 	if err != nil {
 		return nil, err
 	}

cmd/postpolicyform.go

@@ -29,6 +29,7 @@ import (
 	"time"
 
 	"github.com/bcicen/jstream"
+	"github.com/minio/minio-go/v7/pkg/set"
 )
 
 // startWithConds - map which indicates if a given condition supports starts-with policy operator
@@ -121,24 +122,37 @@ type PostPolicyForm struct {
 // Go stdlib doesn't support parsing JSON with duplicate
 // keys, so we need to use this technique to merge the
 // keys.
-func sanitizePolicy(policy string) (io.Reader, error) {
+func sanitizePolicy(r io.Reader) (io.Reader, error) {
 	var buf bytes.Buffer
 	e := json.NewEncoder(&buf)
-	d := jstream.NewDecoder(strings.NewReader(policy), 0)
+	d := jstream.NewDecoder(r, 0).ObjectAsKVS()
+	sset := set.NewStringSet()
 	for mv := range d.Stream() {
-		e.Encode(mv.Value)
+		var kvs jstream.KVS
+		if mv.ValueType == jstream.Object {
+			// This is a JSON object type (that preserves key order)
+			kvs = mv.Value.(jstream.KVS)
+			for _, kv := range kvs {
+				if sset.Contains(kv.Key) {
+					// Reject duplicate conditions or expiration.
+					return nil, fmt.Errorf("input policy has multiple %s, please fix your client code", kv.Key)
+				}
+				sset.Add(kv.Key)
+			}
+			e.Encode(kvs)
+		}
 	}
 	return &buf, d.Err()
 }
 
 // parsePostPolicyForm - Parse JSON policy string into typed PostPolicyForm structure.
-func parsePostPolicyForm(policy string) (PostPolicyForm, error) {
+func parsePostPolicyForm(r io.Reader) (PostPolicyForm, error) {
-	preader, err := sanitizePolicy(policy)
+	reader, err := sanitizePolicy(r)
 	if err != nil {
 		return PostPolicyForm{}, err
 	}
 
-	d := json.NewDecoder(preader)
+	d := json.NewDecoder(reader)
 
 	// Convert po into interfaces and
 	// perform strict type conversion using reflection.

cmd/postpolicyform_test.go

@@ -17,9 +17,11 @@
 package cmd
 
 import (
+	"bytes"
 	"encoding/base64"
 	"fmt"
 	"net/http"
+	"strings"
 	"testing"
 
 	minio "github.com/minio/minio-go/v7"
@@ -40,10 +42,19 @@ func TestParsePostPolicyForm(t *testing.T) {
 			policy:  `{"conditions":[["eq","$bucket","asdf"],["eq","$key","hello.txt"]],"conditions":[["eq","$success_action_status","201"],["eq","$Content-Type","plain/text"],["eq","$success_action_status","201"],["eq","$x-amz-algorithm","AWS4-HMAC-SHA256"],["eq","$x-amz-credential","Q3AM3UQ867SPQQA43P2F/20210315/us-east-1/s3/aws4_request"],["eq","$x-amz-date","20210315T091621Z"]]`,
 			success: false,
 		},
-		// duplicate conditions, shall be parsed properly
+		// duplicate 'expiration' reject
+		{
+			policy: `{"expiration":"2021-03-22T09:16:21.310Z","expiration":"2021-03-22T09:16:21.310Z","conditions":[["eq","$bucket","evil"],["eq","$key","hello.txt"],["eq","$success_action_status","201"],["eq","$Content-Type","plain/text"],["eq","$success_action_status","201"],["eq","$x-amz-algorithm","AWS4-HMAC-SHA256"],["eq","$x-amz-credential","Q3AM3UQ867SPQQA43P2F/20210315/us-east-1/s3/aws4_request"],["eq","$x-amz-date","20210315T091621Z"]]}`,
+		},
+		// duplicate '$bucket' reject
+		{
+			policy:  `{"expiration":"2021-03-22T09:16:21.310Z","conditions":[["eq","$bucket","good"],["eq","$key","hello.txt"]],"conditions":[["eq","$bucket","evil"],["eq","$key","hello.txt"],["eq","$success_action_status","201"],["eq","$Content-Type","plain/text"],["eq","$success_action_status","201"],["eq","$x-amz-algorithm","AWS4-HMAC-SHA256"],["eq","$x-amz-credential","Q3AM3UQ867SPQQA43P2F/20210315/us-east-1/s3/aws4_request"],["eq","$x-amz-date","20210315T091621Z"]]}`,
+			success: false,
+		},
+		// duplicate conditions, reject
 		{
 			policy:  `{"expiration":"2021-03-22T09:16:21.310Z","conditions":[["eq","$bucket","asdf"],["eq","$key","hello.txt"]],"conditions":[["eq","$success_action_status","201"],["eq","$Content-Type","plain/text"],["eq","$success_action_status","201"],["eq","$x-amz-algorithm","AWS4-HMAC-SHA256"],["eq","$x-amz-credential","Q3AM3UQ867SPQQA43P2F/20210315/us-east-1/s3/aws4_request"],["eq","$x-amz-date","20210315T091621Z"]]}`,
-			success: true,
+			success: false,
 		},
 		// no duplicates, shall be parsed properly.
 		{
@@ -55,8 +66,7 @@ func TestParsePostPolicyForm(t *testing.T) {
 	for _, testCase := range testCases {
 		testCase := testCase
 		t.Run("", func(t *testing.T) {
-			_, err := parsePostPolicyForm(testCase.policy)
+			_, err := parsePostPolicyForm(strings.NewReader(testCase.policy))
-			fmt.Println(err)
 			if testCase.success && err != nil {
 				t.Errorf("Expected success but failed with %s", err)
 			}
@@ -136,7 +146,7 @@ func TestPostPolicyForm(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		postPolicyForm, err := parsePostPolicyForm(string(policyBytes))
+		postPolicyForm, err := parsePostPolicyForm(bytes.NewReader(policyBytes))
 		if err != nil {
 			t.Fatal(err)
 		}