CVE-2020-11012: Improper Handling of Exceptional Conditions (CWE-755)
value: MinIO versions before RELEASE.2020-04-23T00-58-49Z have an authentication bypass issue in the MinIO admin API. Given an admin access key, it is possible to perform admin API operations i.e. creating new service accounts for existing access keys - without knowing the admin secret key. This has been fixed and released in version RELEASE.2020-04-23T00-58-49Z.
CWE description: The product does not handle or incorrectly handles an exceptional condition.
CVSS v3 base score: 7.5
Published: 2020-04-23T22:15Z
Repository: https://github.com/minio/minio
Language: Go
Files changed: cmd/auth-handler.go, cmd/auth-handler_test.go
Commit date: 2020-04-22 13:42:01 -0700
Commit message: fix: Add missing return in admin requests auth (#9422)


Diff: diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go
index 88e538740..ee0f8209a 100644
--- a/cmd/auth-handler.go
+++ b/cmd/auth-handler.go
@@ -138,10 +138,11 @@ func validateAdminSignature(ctx context.Context, r *http.Request, region string)
 	}
 	if s3Err != ErrNone {
 		reqInfo := (&logger.ReqInfo{}).AppendTags("requestHeaders", dumpRequest(r))
 		ctx := logger.SetReqInfo(ctx, reqInfo)
 		logger.LogIf(ctx, errors.New(getAPIError(s3Err).Description), logger.Application)
+		return cred, nil, owner, s3Err
 	}
 
 	claims, s3Err := checkClaimsFromToken(r, cred)
 	if s3Err != ErrNone {
 		return cred, nil, owner, s3Err
diff --git a/cmd/auth-handler_test.go b/cmd/auth-handler_test.go
index 62b8af112..92e72e851 100644
--- a/cmd/auth-handler_test.go
+++ b/cmd/auth-handler_test.go
@@ -389,10 +389,11 @@ func TestIsReqAuthenticated(t *testing.T) {
 				t.Fatalf("Test %d: Unexpected S3 error: want %d - got %d (got after reading request %s)", i, testCase.s3Error, s3Error, toAPIError(ctx, err).Code)
 			}
 		}
 	}
 }
+
 func TestCheckAdminRequestAuthType(t *testing.T) {
 	objLayer, fsDir, err := prepareFS()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -423,5 +424,50 @@ func TestCheckAdminRequestAuthType(t *testing.T) {
 		if _, s3Error := checkAdminRequestAuthType(ctx, testCase.Request, iampolicy.AllAdminActions, globalServerRegion); s3Error != testCase.ErrCode {
 			t.Errorf("Test %d: Unexpected s3error returned wanted %d, got %d", i, testCase.ErrCode, s3Error)
 		}
 	}
 }
+
+func TestValidateAdminSignature(t *testing.T) {
+
+	ctx := context.Background()
+
+	objLayer, fsDir, err := prepareFS()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(fsDir)
+
+	if err = newTestConfig(globalMinioDefaultRegion, objLayer); err != nil {
+		t.Fatalf("unable initialize config file, %s", err)
+	}
+
+	creds, err := auth.CreateCredentials("admin", "mypassword")
+	if err != nil {
+		t.Fatalf("unable create credential, %s", err)
+	}
+	globalActiveCred = creds
+
+	testCases := []struct {
+		AccessKey string
+		SecretKey string
+		ErrCode   APIErrorCode
+	}{
+		{"", "", ErrInvalidAccessKeyID},
+		{"admin", "", ErrSignatureDoesNotMatch},
+		{"admin", "wrongpassword", ErrSignatureDoesNotMatch},
+		{"wronguser", "mypassword", ErrInvalidAccessKeyID},
+		{"", "mypassword", ErrInvalidAccessKeyID},
+		{"admin", "mypassword", ErrNone},
+	}
+
+	for i, testCase := range testCases {
+		req := mustNewRequest("GET", "http://localhost:9000/", 0, nil, t)
+		if err := signRequestV4(req, testCase.AccessKey, testCase.SecretKey); err != nil {
+			t.Fatalf("Unable to inititalized new signed http request %s", err)
+		}
+		_, _, _, s3Error := validateAdminSignature(ctx, req, globalMinioDefaultRegion)
+		if s3Error != testCase.ErrCode {
+			t.Errorf("Test %d: Unexpected s3error returned wanted %d, got %d", i+1, testCase.ErrCode, s3Error)
+		}
+	}
+}

Fixed code: 		return cred, nil, owner, s3Err


func TestValidateAdminSignature(t *testing.T) {

	ctx := context.Background()

	objLayer, fsDir, err := prepareFS()
	if err != nil {
		t.Fatal(err)
	}
	defer os.RemoveAll(fsDir)

	if err = newTestConfig(globalMinioDefaultRegion, objLayer); err != nil {
		t.Fatalf("unable initialize config file, %s", err)
	}

	creds, err := auth.CreateCredentials("admin", "mypassword")
	if err != nil {
		t.Fatalf("unable create credential, %s", err)
	}
	globalActiveCred = creds

	testCases := []struct {
		AccessKey string
		SecretKey string
		ErrCode   APIErrorCode
	}{
		{"", "", ErrInvalidAccessKeyID},
		{"admin", "", ErrSignatureDoesNotMatch},
		{"admin", "wrongpassword", ErrSignatureDoesNotMatch},
		{"wronguser", "mypassword", ErrInvalidAccessKeyID},
		{"", "mypassword", ErrInvalidAccessKeyID},
		{"admin", "mypassword", ErrNone},
	}

	for i, testCase := range testCases {
		req := mustNewRequest("GET", "http://localhost:9000/", 0, nil, t)
		if err := signRequestV4(req, testCase.AccessKey, testCase.SecretKey); err != nil {
			t.Fatalf("Unable to inititalized new signed http request %s", err)
		}
		_, _, _, s3Error := validateAdminSignature(ctx, req, globalMinioDefaultRegion)
		if s3Error != testCase.ErrCode {
			t.Errorf("Test %d: Unexpected s3error returned wanted %d, got %d", i+1, testCase.ErrCode, s3Error)
		}
	}
}