CVE-2023-28433: Insufficient Information (NVD-CWE-noinfo)
value: Minio is a Multi-Cloud Object Storage framework. All users on Windows prior to version RELEASE.2023-03-20T20-16-18Z are impacted. MinIO fails to filter the `\` character, which allows for arbitrary object placement across buckets. As a result, a user with low privileges, such as an access key, service account, or STS credential, which only has permission to `PutObject` in a specific bucket, can create an admin user. This issue is patched in RELEASE.2023-03-20T20-16-18Z. There are no known workarounds.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 8.8
Published: 2023-03-22T21:15Z
Repository: https://github.com/minio/minio
Language: Go
Files changed: cmd/object-api-multipart_test.go, cmd/object-api-utils.go, cmd/object-api-utils_test.go
Commit date: 2023-03-20 13:16:00 -0700
Commit message: reject object names with '\' on windows (#16856)


Diff: diff --git a/cmd/object-api-multipart_test.go b/cmd/object-api-multipart_test.go
index efb152bcc4..80905f9ee9 100644
--- a/cmd/object-api-multipart_test.go
+++ b/cmd/object-api-multipart_test.go
@@ -20,20 +20,24 @@ package cmd
 import (
 	"bytes"
 	"context"
 	"fmt"
 	"reflect"
+	"runtime"
 	"strings"
 	"testing"
 
 	"github.com/dustin/go-humanize"
 	"github.com/minio/minio/internal/config/storageclass"
 	"github.com/minio/minio/internal/hash"
 )
 
 // Wrapper for calling NewMultipartUpload tests for both Erasure multiple disks and single node setup.
 func TestObjectNewMultipartUpload(t *testing.T) {
+	if runtime.GOOS == globalWindowsOSName {
+		t.Skip()
+	}
 	ExecObjectLayerTest(t, testObjectNewMultipartUpload)
 }
 
 // Tests validate creation of new multipart upload instance.
 func testObjectNewMultipartUpload(obj ObjectLayer, instanceType string, t TestErrHandler) {
@@ -108,13 +112,22 @@ func testObjectAbortMultipartUpload(obj ObjectLayer, instanceType string, t Test
 		expectedErrType error
 	}{
 		{"--", object, uploadID, BucketNotFound{}},
 		{"foo", object, uploadID, BucketNotFound{}},
 		{bucket, object, "foo-foo", InvalidUploadID{}},
-		{bucket, "\\", uploadID, InvalidUploadID{}},
 		{bucket, object, uploadID, nil},
 	}
+
+	if runtime.GOOS != globalWindowsOSName {
+		abortTestCases = append(abortTestCases, struct {
+			bucketName      string
+			objName         string
+			uploadID        string
+			expectedErrType error
+		}{bucket, "\\", uploadID, InvalidUploadID{}})
+	}
+
 	// Iterating over creatPartCases to generate multipart chunks.
 	for i, testCase := range abortTestCases {
 		err = obj.AbortMultipartUpload(context.Background(), testCase.bucketName, testCase.objName, testCase.uploadID, opts)
 		if testCase.expectedErrType == nil && err != nil {
 			t.Errorf("Test %d, unexpected err is received: %v, expected:%v\n", i+1, err, testCase.expectedErrType)
diff --git a/cmd/object-api-utils.go b/cmd/object-api-utils.go
index c4f5373515..3ea5a9fad4 100644
--- a/cmd/object-api-utils.go
+++ b/cmd/object-api-utils.go
@@ -172,14 +172,11 @@ func IsValidObjectPrefix(object string) bool {
 	}
 	// This is valid for AWS S3 but it will never
 	// work with file systems, we will reject here
 	// to return object name invalid rather than
 	// a cryptic error from the file system.
-	if strings.ContainsRune(object, 0) {
-		return false
-	}
-	return true
+	return !strings.ContainsRune(object, 0)
 }
 
 // checkObjectNameForLengthAndSlash -check for the validity of object name length and prefis as slash
 func checkObjectNameForLengthAndSlash(bucket, object string) error {
 	// Check for the length of object name
@@ -197,11 +194,11 @@ func checkObjectNameForLengthAndSlash(bucket, object string) error {
 		}
 	}
 	if runtime.GOOS == globalWindowsOSName {
 		// Explicitly disallowed characters on windows.
 		// Avoids most problematic names.
-		if strings.ContainsAny(object, `:*?"|<>`) {
+		if strings.ContainsAny(object, `\:*?"|<>`) {
 			return ObjectNameInvalid{
 				Bucket: bucket,
 				Object: object,
 			}
 		}
diff --git a/cmd/object-api-utils_test.go b/cmd/object-api-utils_test.go
index 349a00802b..114b26a21d 100644
--- a/cmd/object-api-utils_test.go
+++ b/cmd/object-api-utils_test.go
@@ -17,23 +17,79 @@
 
 package cmd
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"io"
 	"net/http"
+	"net/http/httptest"
 	"reflect"
+	"runtime"
 	"strconv"
 	"testing"
 
 	"github.com/klauspost/compress/s2"
+	"github.com/minio/minio/internal/auth"
 	"github.com/minio/minio/internal/config/compress"
 	"github.com/minio/minio/internal/crypto"
 	"github.com/minio/pkg/trie"
 )
 
+// Wrapper
+func TestPathTraversalExploit(t *testing.T) {
+	if runtime.GOOS != globalWindowsOSName {
+		t.Skip()
+	}
+	defer DetectTestLeak(t)()
+	ExecExtendedObjectLayerAPITest(t, testPathTraversalExploit, []string{"PutObject"})
+}
+
+// testPathTraversal exploit test, exploits path traversal on windows
+// with following object names "\\../.minio.sys/config/iam/${username}/identity.json"
+// #16852
+func testPathTraversalExploit(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler,
+	credentials auth.Credentials, t *testing.T,
+) {
+	if err := newTestConfig(globalMinioDefaultRegion, obj); err != nil {
+		t.Fatalf("Initializing config.json failed")
+	}
+
+	objectName := `\../.minio.sys/config/hello.txt`
+
+	// initialize HTTP NewRecorder, this records any mutations to response writer inside the handler.
+	rec := httptest.NewRecorder()
+	// construct HTTP request for Get Object end point.
+	req, err := newTestSignedRequestV4(http.MethodPut, getPutObjectURL("", bucketName, objectName),
+		int64(5), bytes.NewReader([]byte("hello")), credentials.AccessKey, credentials.SecretKey, map[string]string{})
+	if err != nil {
+		t.Fatalf("failed to create HTTP request for Put Object: <ERROR> %v", err)
+	}
+
+	// Since `apiRouter` satisfies `http.Handler` it has a ServeHTTP to execute the logic ofthe handler.
+	// Call the ServeHTTP to execute the handler.
+	apiRouter.ServeHTTP(rec, req)
+
+	ctx, cancel := context.WithCancel(GlobalContext)
+	defer cancel()
+
+	// Now check if we actually wrote to backend (regardless of the response
+	// returned by the server).
+	z := obj.(*erasureServerPools)
+	xl := z.serverPools[0].sets[0]
+	erasureDisks := xl.getDisks()
+	parts, errs := readAllFileInfo(ctx, erasureDisks, bucketName, objectName, "", false)
+	for i := range parts {
+		if errs[i] == nil {
+			if parts[i].Name == objectName {
+				t.Errorf("path traversal allowed to allow writing to minioMetaBucket: %s", instanceType)
+			}
+		}
+	}
+}
+
 // Tests validate bucket name.
 func TestIsValidBucketName(t *testing.T) {
 	testCases := []struct {
 		bucketName string
 		shouldPass bool

Vulnerable code: 		{bucket, "\\", uploadID, InvalidUploadID{}},
	if strings.ContainsRune(object, 0) {
		return false
	}
	return true
		if strings.ContainsAny(object, `:*?"|<>`) {
Fixed code: 	"runtime"
	if runtime.GOOS == globalWindowsOSName {
		t.Skip()
	}

	if runtime.GOOS != globalWindowsOSName {
		abortTestCases = append(abortTestCases, struct {
			bucketName      string
			objName         string
			uploadID        string
			expectedErrType error
		}{bucket, "\\", uploadID, InvalidUploadID{}})
	}

	return !strings.ContainsRune(object, 0)
		if strings.ContainsAny(object, `\:*?"|<>`) {
	"context"
	"net/http/httptest"
	"runtime"
	"github.com/minio/minio/internal/auth"
// Wrapper
func TestPathTraversalExploit(t *testing.T) {
	if runtime.GOOS != globalWindowsOSName {
		t.Skip()
	}
	defer DetectTestLeak(t)()
	ExecExtendedObjectLayerAPITest(t, testPathTraversalExploit, []string{"PutObject"})
}

// testPathTraversal exploit test, exploits path traversal on windows
// with following object names "\\../.minio.sys/config/iam/${username}/identity.json"
// #16852
func testPathTraversalExploit(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler,
	credentials auth.Credentials, t *testing.T,
) {
	if err := newTestConfig(globalMinioDefaultRegion, obj); err != nil {
		t.Fatalf("Initializing config.json failed")
	}

	objectName := `\../.minio.sys/config/hello.txt`

	// initialize HTTP NewRecorder, this records any mutations to response writer inside the handler.
	rec := httptest.NewRecorder()
	// construct HTTP request for Get Object end point.
	req, err := newTestSignedRequestV4(http.MethodPut, getPutObjectURL("", bucketName, objectName),
		int64(5), bytes.NewReader([]byte("hello")), credentials.AccessKey, credentials.SecretKey, map[string]string{})
	if err != nil {
		t.Fatalf("failed to create HTTP request for Put Object: <ERROR> %v", err)
	}

	// Since `apiRouter` satisfies `http.Handler` it has a ServeHTTP to execute the logic ofthe handler.
	// Call the ServeHTTP to execute the handler.
	apiRouter.ServeHTTP(rec, req)

	ctx, cancel := context.WithCancel(GlobalContext)
	defer cancel()

	// Now check if we actually wrote to backend (regardless of the response
	// returned by the server).
	z := obj.(*erasureServerPools)
	xl := z.serverPools[0].sets[0]
	erasureDisks := xl.getDisks()
	parts, errs := readAllFileInfo(ctx, erasureDisks, bucketName, objectName, "", false)
	for i := range parts {
		if errs[i] == nil {
			if parts[i].Name == objectName {
				t.Errorf("path traversal allowed to allow writing to minioMetaBucket: %s", instanceType)
			}
		}
	}
}