diff --git a/cmd/object-api-multipart_test.go b/cmd/object-api-multipart_test.go index efb152bcc..80905f9ee 100644 --- a/cmd/object-api-multipart_test.go +++ b/cmd/object-api-multipart_test.go @@ -22,6 +22,7 @@ import ( "context" "fmt" "reflect" + "runtime" "strings" "testing" @@ -32,6 +33,9 @@ import ( // 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) } @@ -110,9 +114,18 @@ func testObjectAbortMultipartUpload(obj ObjectLayer, instanceType string, t Test {"--", 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) diff --git a/cmd/object-api-utils.go b/cmd/object-api-utils.go index c4f537351..3ea5a9fad 100644 --- a/cmd/object-api-utils.go +++ b/cmd/object-api-utils.go @@ -174,10 +174,7 @@ func IsValidObjectPrefix(object string) bool { // 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 @@ -199,7 +196,7 @@ 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 349a00802..114b26a21 100644 --- a/cmd/object-api-utils_test.go +++ b/cmd/object-api-utils_test.go @@ -19,19 +19,75 @@ 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: %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 {