fix: only treat a trailing ? as optional in MatchSimple

policy/bucket-policy_test.go

@@ -216,6 +216,32 @@ func TestBucketPolicyIsAllowed(t *testing.T) {
 	}
 }
 
+func TestBucketPolicyIsAllowedAnonymousPrincipalPattern(t *testing.T) {
+	args := BucketPolicyArgs{
+		Action:          GetObjectAction,
+		BucketName:      "mybucket",
+		ObjectName:      "myobject",
+		ConditionValues: map[string][]string{},
+	}
+	for _, principal := range []string{"?alice", "*?alice", "?arn:aws:iam::123456789012:user/alice"} {
+		p := BucketPolicy{
+			Version: DefaultVersion,
+			Statements: []BPStatement{
+				NewBPStatement("",
+					Allow,
+					NewPrincipal(principal),
+					NewActionSet(GetObjectAction),
+					NewResourceSet(NewResource("mybucket/*")),
+					condition.NewFunctions(),
+				),
+			},
+		}
+		if p.IsAllowed(args) {
+			t.Errorf("principal %q: anonymous GetObject allowed, want denied", principal)
+		}
+	}
+}
+
 func TestBucketPolicyIsEmpty(t *testing.T) {
 	case1Policy := BucketPolicy{
 		Version: DefaultVersion,

policy/principal_test.go

@@ -100,6 +100,10 @@ func TestPrincipalMatch(t *testing.T) {
 		{NewPrincipal("*"), "AccountNumber", true},
 		{NewPrincipal("arn:aws:iam::*"), "arn:aws:iam::AccountNumber:root", true},
 		{NewPrincipal("arn:aws:iam::AccountNumber:*"), "arn:aws:iam::TestAccountNumber:root", false},
+		{NewPrincipal("*"), "", true},
+		{NewPrincipal("?alice"), "", false},
+		{NewPrincipal("*?alice"), "", false},
+		{NewPrincipal("?alice"), "xalice", true},
 	}
 
 	for i, testCase := range testCases {

wildcard/match.go

@@ -23,8 +23,9 @@ import (
 
 // MatchSimple - finds whether the text matches/satisfies the pattern string.
 // supports '*' wildcard in the pattern and ? for single characters.
-// Only difference to Match is that `?` at the end is optional,
-// meaning `a?` pattern will match name `a`.
+// Only difference to Match is that a `?` at the end is optional,
+// meaning `a?` pattern will match name `a`. A `?` followed by anything
+// other than `?` or `*` still matches exactly one character.
 func MatchSimple(pattern, name string) bool {
 	if pattern == "" {
 		return name == pattern
@@ -35,10 +36,11 @@ func MatchSimple(pattern, name string) bool {
 	if deepMatchRune(name, pattern) {
 		return true
 	}
-	// Reaching a '?' with name already exhausted succeeds, and succeeds for
-	// the whole pattern, so the pattern also matches whenever any prefix of it
-	// ending at a '?' consumes name exactly.
-	if !strings.ContainsRune(pattern, '?') {
+	// An optional '?' is one that only '?' and '*' follow, so the pattern also
+	// matches whenever a prefix of it ending at one of those consumes name
+	// exactly.
+	tail := len(strings.TrimRight(pattern, "*?"))
+	if strings.IndexByte(pattern[tail:], '?') < 0 {
 		return false
 	}
 	// A prefix holding no '*' consumes exactly one byte of name per byte of
@@ -53,7 +55,7 @@ func MatchSimple(pattern, name string) bool {
 	// too long to consume name exactly and the walk can stop.
 	lits := 0
 	for i := range len(pattern) {
-		if pattern[i] == '?' && (i > firstStar || i == len(name)) &&
+		if pattern[i] == '?' && i >= tail && (i > firstStar || i == len(name)) &&
 			deepMatchRune(name, pattern[:i]) {
 			return true
 		}

wildcard/match_equivalence_test.go

@@ -34,7 +34,7 @@ func oldDeepMatchRune(str, pattern string, simple bool) bool {
 			}
 		case '?':
 			if len(str) == 0 {
-				return simple
+				return simple && strings.Trim(pattern, "*?") == ""
 			}
 		case '*':
 			return len(pattern) == 1 ||

wildcard/match_test.go

@@ -544,6 +544,22 @@ func TestMatchSimple(t *testing.T) {
 			text:    "a",
 			matched: true,
 		},
+		{pattern: "?alice", text: "", matched: false},
+		{pattern: "?alice", text: "xalice", matched: true},
+		{pattern: "*?alice", text: "", matched: false},
+		{pattern: "a?b", text: "a", matched: false},
+		{pattern: "a?b", text: "ab", matched: false},
+		{pattern: "a?b", text: "axb", matched: true},
+		{pattern: "a?*b", text: "a", matched: false},
+		{pattern: "a*?b", text: "a", matched: false},
+		{pattern: "?", text: "", matched: true},
+		{pattern: "*?", text: "", matched: true},
+		{pattern: "?*", text: "", matched: true},
+		{pattern: "a??", text: "a", matched: true},
+		{pattern: "a??", text: "ab", matched: true},
+		{pattern: "a??", text: "abcd", matched: false},
+		{pattern: "a?*?", text: "a", matched: true},
+		{pattern: "a*b?", text: "axb", matched: true},
 	}
 	// Iterating over the test cases, call the function under test and assert the output.
 	for i, testCase := range testCases {