CVE-2021-41137: Other (NVD-CWE-Other)
value: Minio is a Kubernetes native application for cloud storage. All users on release `RELEASE.2021-10-10T16-53-30Z` are affected by a vulnerability that involves bypassing policy restrictions on regular users. Normally, checkKeyValid() should return owner true for rootCreds. In the affected version, policy restriction did not work properly for users who did not have service (svc) or security token service (STS) accounts. This issue is fixed in `RELEASE.2021-10-13T00-23-17Z`. A downgrade back to release `RELEASE.2021-10-08T23-58-24Z` is available as a workaround.
CWE description: NVD is only using a subset of CWE for mapping instead of the entire CWE, and the weakness type is not covered by that subset.
CVSS v3 base score: 8.8
Published: 2021-10-13T14:15Z
Repository: https://github.com/minio/minio
Language: Go
Files changed: cmd/signature-v4-utils.go, cmd/signature-v4-utils_test.go
Commit date: 2021-10-12 13:18:02 -0700
Commit message: checkKeyValid() should return owner true for rootCreds (#13422)

Looks like policy restriction was not working properly
for normal users when they are not svc or STS accounts.

- svc accounts are now properly fixed to get
  right permissions when its inherited, so
  we do not have to set 'owner = true'

- sts accounts have always been using right
  permissions, do not need an explicit lookup

- regular users always have proper policy mapping
Diff: diff --git a/cmd/signature-v4-utils.go b/cmd/signature-v4-utils.go
index b28a380207..9569688a26 100644
--- a/cmd/signature-v4-utils.go
+++ b/cmd/signature-v4-utils.go
@@ -29,11 +29,10 @@ import (
 	"strings"
 
 	"github.com/minio/minio/internal/auth"
 	xhttp "github.com/minio/minio/internal/http"
 	"github.com/minio/minio/internal/logger"
-	iampolicy "github.com/minio/pkg/iam/policy"
 )
 
 // http Header "x-amz-content-sha256" == "UNSIGNED-PAYLOAD" indicates that the
 // client did not calculate sha256 of the payload.
 const unsignedPayload = "UNSIGNED-PAYLOAD"
@@ -148,12 +147,11 @@ func checkKeyValid(r *http.Request, accessKey string) (auth.Credentials, bool, A
 		// to check for IAM users otherwise its okay for clients
 		// to retry with 503 errors when server is coming up.
 		return auth.Credentials{}, false, ErrServerNotInitialized
 	}
 
-	var owner = true
-	var cred = globalActiveCred
+	cred := globalActiveCred
 	if cred.AccessKey != accessKey {
 		// Check if the access key is part of users credentials.
 		ucred, ok := globalIAMSys.GetUser(accessKey)
 		if !ok {
 			return cred, false, ErrInvalidAccessKeyID
@@ -163,22 +161,13 @@ func checkKeyValid(r *http.Request, accessKey string) (auth.Credentials, bool, A
 
 	claims, s3Err := checkClaimsFromToken(r, cred)
 	if s3Err != ErrNone {
 		return cred, false, s3Err
 	}
+	cred.Claims = claims
 
-	if len(claims) > 0 {
-		cred.Claims = claims
-
-		// Now check if we have a sessionPolicy.
-		if _, ok := claims[iampolicy.SessionPolicyName]; ok {
-			owner = false
-		} else {
-			owner = cred.AccessKey == cred.ParentUser
-		}
-	}
-
+	owner := cred.AccessKey == globalActiveCred.AccessKey
 	return cred, owner, ErrNone
 }
 
 // sumHMAC calculate hmac between two input byte array.
 func sumHMAC(key []byte, data []byte) []byte {
diff --git a/cmd/signature-v4-utils_test.go b/cmd/signature-v4-utils_test.go
index d56a20e707..80000c2aa5 100644
--- a/cmd/signature-v4-utils_test.go
+++ b/cmd/signature-v4-utils_test.go
@@ -16,16 +16,79 @@
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 package cmd
 
 import (
+	"context"
 	"net/http"
+	"os"
 	"testing"
 
+	"github.com/minio/madmin-go"
+	"github.com/minio/minio/internal/auth"
 	xhttp "github.com/minio/minio/internal/http"
 )
 
+func TestCheckValid(t *testing.T) {
+	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)
+	}
+
+	newAllSubsystems()
+
+	initAllSubsystems(context.Background(), objLayer)
+
+	globalIAMSys.InitStore(objLayer)
+
+	req, err := newTestRequest(http.MethodGet, "http://example.com:9000/bucket/object", 0, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if err = signRequestV4(req, globalActiveCred.AccessKey, globalActiveCred.SecretKey); err != nil {
+		t.Fatal(err)
+	}
+
+	_, owner, s3Err := checkKeyValid(req, globalActiveCred.AccessKey)
+	if s3Err != ErrNone {
+		t.Fatalf("Unexpected failure with %v", errorCodes.ToAPIErr(s3Err))
+	}
+
+	if !owner {
+		t.Fatalf("Expected owner to be 'true', found %t", owner)
+	}
+
+	_, _, s3Err = checkKeyValid(req, "does-not-exist")
+	if s3Err != ErrInvalidAccessKeyID {
+		t.Fatalf("Expected error 'ErrInvalidAccessKeyID', found %v", s3Err)
+	}
+
+	ucreds, err := auth.CreateCredentials("myuser1", "mypassword1")
+	if err != nil {
+		t.Fatalf("unable create credential, %s", err)
+	}
+
+	globalIAMSys.CreateUser(ucreds.AccessKey, madmin.UserInfo{
+		SecretKey: ucreds.SecretKey,
+		Status:    madmin.AccountEnabled,
+	})
+
+	_, owner, s3Err = checkKeyValid(req, ucreds.AccessKey)
+	if s3Err != ErrNone {
+		t.Fatalf("Unexpected failure with %v", errorCodes.ToAPIErr(s3Err))
+	}
+
+	if owner {
+		t.Fatalf("Expected owner to be 'false', found %t", owner)
+	}
+}
+
 // TestSkipContentSha256Cksum - Test validate the logic which decides whether
 // to skip checksum validation based on the request header.
 func TestSkipContentSha256Cksum(t *testing.T) {
 	testCases := []struct {
 		inputHeaderKey   string

Vulnerable code: 	iampolicy "github.com/minio/pkg/iam/policy"
	var owner = true
	var cred = globalActiveCred
	if len(claims) > 0 {
		cred.Claims = claims

		// Now check if we have a sessionPolicy.
		if _, ok := claims[iampolicy.SessionPolicyName]; ok {
			owner = false
		} else {
			owner = cred.AccessKey == cred.ParentUser
		}
	}

Fixed code: 	cred := globalActiveCred
	cred.Claims = claims
	owner := cred.AccessKey == globalActiveCred.AccessKey
	"context"
	"os"
	"github.com/minio/madmin-go"
	"github.com/minio/minio/internal/auth"
func TestCheckValid(t *testing.T) {
	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)
	}

	newAllSubsystems()

	initAllSubsystems(context.Background(), objLayer)

	globalIAMSys.InitStore(objLayer)

	req, err := newTestRequest(http.MethodGet, "http://example.com:9000/bucket/object", 0, nil)
	if err != nil {
		t.Fatal(err)
	}

	if err = signRequestV4(req, globalActiveCred.AccessKey, globalActiveCred.SecretKey); err != nil {
		t.Fatal(err)
	}

	_, owner, s3Err := checkKeyValid(req, globalActiveCred.AccessKey)
	if s3Err != ErrNone {
		t.Fatalf("Unexpected failure with %v", errorCodes.ToAPIErr(s3Err))
	}

	if !owner {
		t.Fatalf("Expected owner to be 'true', found %t", owner)
	}

	_, _, s3Err = checkKeyValid(req, "does-not-exist")
	if s3Err != ErrInvalidAccessKeyID {
		t.Fatalf("Expected error 'ErrInvalidAccessKeyID', found %v", s3Err)
	}

	ucreds, err := auth.CreateCredentials("myuser1", "mypassword1")
	if err != nil {
		t.Fatalf("unable create credential, %s", err)
	}

	globalIAMSys.CreateUser(ucreds.AccessKey, madmin.UserInfo{
		SecretKey: ucreds.SecretKey,
		Status:    madmin.AccountEnabled,
	})

	_, owner, s3Err = checkKeyValid(req, ucreds.AccessKey)
	if s3Err != ErrNone {
		t.Fatalf("Unexpected failure with %v", errorCodes.ToAPIErr(s3Err))
	}

	if owner {
		t.Fatalf("Expected owner to be 'false', found %t", owner)
	}
}