From 44e4bdc6f44f8c7c20f8c33e005d4918328e6cdb Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Mon, 18 Oct 2021 08:38:33 -0700
Subject: [PATCH] restrict multi object delete > 1000 objects (#13454)

AWS S3 returns error if > 1000 objects are sent
per MultiObject delete request, we should comply
no reason to not comply.
---
 cmd/api-response.go    |  2 +-
 cmd/bucket-handlers.go |  4 ++--
 cmd/local-locker.go    | 21 +++++++++++++++------
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/cmd/api-response.go b/cmd/api-response.go
index 2c63c5d57..d3c79114b 100644
--- a/cmd/api-response.go
+++ b/cmd/api-response.go
@@ -39,7 +39,7 @@ const (
 	// RFC3339 a subset of the ISO8601 timestamp format. e.g 2014-04-29T18:30:38Z
 	iso8601TimeFormat = "2006-01-02T15:04:05.000Z" // Reply date format with nanosecond precision.
 	maxObjectList     = 1000                       // Limit number of objects in a listObjectsResponse/listObjectsVersionsResponse.
-	maxDeleteList     = 10000                      // Limit number of objects deleted in a delete call.
+	maxDeleteList     = 1000                       // Limit number of objects deleted in a delete call.
 	maxUploadsList    = 10000                      // Limit number of uploads in a listUploadsResponse.
 	maxPartsList      = 10000                      // Limit number of parts in a listPartsResponse.
 )
diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go
index 8d2ad96fe..c0e1afebb 100644
--- a/cmd/bucket-handlers.go
+++ b/cmd/bucket-handlers.go
@@ -442,8 +442,8 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
 		deleteObjectsFn = api.CacheAPI().DeleteObjects
 	}
 
-	// Return Malformed XML as S3 spec if the list of objects is empty
-	if len(deleteObjects.Objects) == 0 {
+	// Return Malformed XML as S3 spec if the number of objects is empty
+	if len(deleteObjects.Objects) == 0 || len(deleteObjects.Objects) > maxDeleteList {
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMalformedXML), r.URL)
 		return
 	}
diff --git a/cmd/local-locker.go b/cmd/local-locker.go
index 17ac4d05c..25bcca86e 100644
--- a/cmd/local-locker.go
+++ b/cmd/local-locker.go
@@ -20,7 +20,6 @@ package cmd
 import (
 	"context"
 	"fmt"
-	"math"
 	"strconv"
 	"sync"
 	"time"
@@ -81,6 +80,10 @@ func (l *localLocker) canTakeLock(resources ...string) bool {
 }
 
 func (l *localLocker) Lock(ctx context.Context, args dsync.LockArgs) (reply bool, err error) {
+	if len(args.Resources) > maxDeleteList {
+		return false, fmt.Errorf("internal error: localLocker.Lock called with more than %d resources", maxDeleteList)
+	}
+
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
 
@@ -117,6 +120,10 @@ func formatUUID(s string, idx int) string {
 }
 
 func (l *localLocker) Unlock(_ context.Context, args dsync.LockArgs) (reply bool, err error) {
+	if len(args.Resources) > maxDeleteList {
+		return false, fmt.Errorf("internal error: localLocker.Unlock called with more than %d resources", maxDeleteList)
+	}
+
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
 	err = nil
@@ -265,7 +272,7 @@ func (l *localLocker) ForceUnlock(ctx context.Context, args dsync.LockArgs) (rep
 					lris, ok := l.lockMap[resource]
 					if !ok {
 						// Just to be safe, delete uuids.
-						for idx := 0; idx < math.MaxInt32; idx++ {
+						for idx := 0; idx < maxDeleteList; idx++ {
 							mapID := formatUUID(uid, idx)
 							if _, ok := l.lockUID[mapID]; !ok {
 								break
@@ -282,14 +289,15 @@ func (l *localLocker) ForceUnlock(ctx context.Context, args dsync.LockArgs) (rep
 
 		idx := 0
 		for {
-			resource, ok := l.lockUID[formatUUID(args.UID, idx)]
+			mapID := formatUUID(args.UID, idx)
+			resource, ok := l.lockUID[mapID]
 			if !ok {
 				return idx > 0, nil
 			}
 			lris, ok := l.lockMap[resource]
 			if !ok {
 				// Unexpected  inconsistency, delete.
-				delete(l.lockUID, formatUUID(args.UID, idx))
+				delete(l.lockUID, mapID)
 				idx++
 				continue
 			}
@@ -315,10 +323,11 @@ func (l *localLocker) Refresh(ctx context.Context, args dsync.LockArgs) (refresh
 		}
 		idx := 0
 		for {
+			mapID := formatUUID(args.UID, idx)
 			lris, ok := l.lockMap[resource]
 			if !ok {
 				// Inconsistent. Delete UID.
-				delete(l.lockUID, formatUUID(args.UID, idx))
+				delete(l.lockUID, mapID)
 				return idx > 0, nil
 			}
 			for i := range lris {
@@ -327,7 +336,7 @@ func (l *localLocker) Refresh(ctx context.Context, args dsync.LockArgs) (refresh
 				}
 			}
 			idx++
-			resource, ok = l.lockUID[formatUUID(args.UID, idx)]
+			resource, ok = l.lockUID[mapID]
 			if !ok {
 				// No more resources for UID, but we did update at least one.
 				return true, nil