Repository: minio/minio (language: go)

Issue:
Pre-signed URL won't work with tokens being too big
## NOTE
If this case is urgent, please subscribe to [Subnet](https://min.io/pricing) so that our 24/7 support team may help you faster.

<!--- Provide a general summary of the issue in the Title above -->

## Expected Behavior
<!--- If you're describing a bug, tell us what should happen -->
<!--- If you're suggesting a change/improvement, tell us how it should work -->

Pre-signed URL should still be working for (in our case) download.

## Current Behavior
<!--- If describing a bug, tell us what happens instead of the expected behavior -->
<!--- If suggesting a change/improvement, explain the difference from current behavior -->

 Won't work, throwing a "XMinioInvalidResourceName" error with the following message : "Resource name contains bad components such as ".." or "."".


## Possible Solution
<!--- Not obligatory, but suggest a fix/reason for the bug, -->
<!--- or ideas how to implement the addition or change -->

On April 2024, a fix to limit the URL to be higher than the max size Linux for a path was introduced : https://github.com/minio/minio/pull/19466/files#diff-28dd2e2e4af0c5a25600cd06909e22a80dc4ad56d203f29f3073110d12240b3d

This also prevents to inject any parameter in the URL, such as the ones required for pre-signed URL. What about limiting the check until the first '?' token is read ?

## Steps to Reproduce (for bugs)
<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug. Include code to reproduce, if relevant -->
<!--- and make sure you have followed https://github.com/minio/minio/tree/release/docs/debugging to capture relevant logs -->

1. Create a RBAC that contains quite a big token
2. Try to download using a pre-signed URL 

## Context
<!--- How has this issue affected you? What are you trying to accomplish? -->
<!--- Providing context helps us come up with a solution that is most useful in the real world -->

Just downloading files and sending an URL to the browser and automatically download it without setting headers.




Discussion / hints:
Yeah, that seems to be an unintended sideeffect. I will send in a fix.

Fix patch:
diff --git a/cmd/generic-handlers.go b/cmd/generic-handlers.go
index 67b906391eee0..55986fdd8177f 100644
--- a/cmd/generic-handlers.go
+++ b/cmd/generic-handlers.go
@@ -22,11 +22,11 @@ import (
 	"net"
 	"net/http"
 	"path"
-	"path/filepath"
 	"runtime/debug"
 	"strings"
 	"sync/atomic"
 	"time"
+	"unicode"
 
 	"github.com/dustin/go-humanize"
 	"github.com/minio/minio-go/v7/pkg/s3utils"
@@ -293,12 +293,6 @@ func parseAmzDateHeader(req *http.Request) (time.Time, APIErrorCode) {
 	return time.Time{}, ErrMissingDateHeader
 }
 
-// Bad path components to be rejected by the path validity handler.
-const (
-	dotdotComponent = ".."
-	dotComponent    = "."
-)
-
 func hasBadHost(host string) error {
 	if globalIsCICD && strings.TrimSpace(host) == "" {
 		// under CI/CD test setups ignore empty hosts as invalid hosts
@@ -311,21 +305,41 @@ func hasBadHost(host string) error {
 // Check if the incoming path has bad path components,
 // such as ".." and "."
 func hasBadPathComponent(path string) bool {
-	if len(path) > 4096 {
-		// path cannot be greater than Linux PATH_MAX
-		// this is to avoid a busy loop, that can happen
-		// if the caller sends path of following style
-		// a/a/a/a/a/a/a/a...
+	n := len(path)
+	if n > 32<<10 {
+		// At 32K we are beyond reasonable.
 		return true
 	}
-	path = filepath.ToSlash(strings.TrimSpace(path)) // For windows '\' must be converted to '/'
-	for _, p := range strings.Split(path, SlashSeparator) {
-		switch strings.TrimSpace(p) {
-		case dotdotComponent:
+	i := 0
+	// Skip leading slashes (for sake of Windows \ is included as well)
+	for i < n && (path[i] == SlashSeparatorChar || path[i] == '\\') {
+		i++
+	}
+
+	for i < n {
+		// Find the next segment
+		start := i
+		for i < n && path[i] != SlashSeparatorChar && path[i] != '\\' {
+			i++
+		}
+
+		// Trim whitespace of segment
+		segmentStart, segmentEnd := start, i
+		for segmentStart < segmentEnd && unicode.IsSpace(rune(path[segmentStart])) {
+			segmentStart++
+		}
+		for segmentEnd > segmentStart && unicode.IsSpace(rune(path[segmentEnd-1])) {
+			segmentEnd--
+		}
+
+		// Check for ".." or "."
+		switch {
+		case segmentEnd-segmentStart == 2 && path[segmentStart] == '.' && path[segmentStart+1] == '.':
 			return true
-		case dotComponent:
+		case segmentEnd-segmentStart == 1 && path[segmentStart] == '.':
 			return true
 		}
+		i++
 	}
 	return false
 }


Test patch:
diff --git a/cmd/generic-handlers_test.go b/cmd/generic-handlers_test.go
index f6cc6e07e607d..03813ebc06334 100644
--- a/cmd/generic-handlers_test.go
+++ b/cmd/generic-handlers_test.go
@@ -22,6 +22,7 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/minio/minio/internal/crypto"
@@ -184,3 +185,27 @@ func TestSSETLSHandler(t *testing.T) {
 		}
 	}
 }
+
+func Benchmark_hasBadPathComponent(t *testing.B) {
+	tests := []struct {
+		name  string
+		input string
+		want  bool
+	}{
+		{name: "empty", input: "", want: false},
+		{name: "backslashes", input: `\a\a\ \\  \\\\\\\`, want: false},
+		{name: "long", input: strings.Repeat("a/", 2000), want: false},
+		{name: "long-fail", input: strings.Repeat("a/", 2000) + "../..", want: true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(b *testing.B) {
+			b.SetBytes(int64(len(tt.input)))
+			b.ReportAllocs()
+			for i := 0; i < b.N; i++ {
+				if got := hasBadPathComponent(tt.input); got != tt.want {
+					t.Fatalf("hasBadPathComponent() = %v, want %v", got, tt.want)
+				}
+			}
+		})
+	}
+}