From 319f446113e4c7bd92ff1b3732fc9613dcabe1a0 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Sun, 4 Oct 2026 16:15:32 +0530 Subject: [PATCH 1/2] fix(iam): anchor policy wildcards and support ? --- providers/aws/iam/condition.go | 13 +- providers/aws/iam/iam.go | 33 +--- providers/aws/iam/iam_test.go | 2 +- providers/aws/iam/simulate.go | 41 ++++- providers/aws/iam/trust_policy.go | 2 +- providers/aws/iam/wildcard.go | 72 ++++++++ providers/aws/iam/wildcard_match_test.go | 208 +++++++++++++++++++++++ 7 files changed, 323 insertions(+), 48 deletions(-) create mode 100644 providers/aws/iam/wildcard.go create mode 100644 providers/aws/iam/wildcard_match_test.go diff --git a/providers/aws/iam/condition.go b/providers/aws/iam/condition.go index af0b82e27..efc73ea2d 100644 --- a/providers/aws/iam/condition.go +++ b/providers/aws/iam/condition.go @@ -188,14 +188,14 @@ func stringOp(op string) (cmp func(ctxVal, policyVal string) bool, negate, ok bo } } -// arnOp handles ARN operators. ArnEquals and ArnLike both allow the * wildcard -// (AWS treats them equivalently apart from documented case handling). +// arnOp handles ARN operators. ArnEquals and ArnLike behave the same in IAM: +// both match component by component and allow '*' and '?' in each component. func arnOp(op string) (cmp func(ctxVal, policyVal string) bool, negate, ok bool) { switch op { case "ArnEquals", "ArnLike": - return strLike, false, true + return arnMatch, false, true case "ArnNotEquals", "ArnNotLike": - return strLike, true, true + return arnMatch, true, true default: return nil, false, false } @@ -214,8 +214,9 @@ func ipOp(op string) (cmp func(ctxVal, policyVal string) bool, negate, ok bool) func strEqual(ctxVal, policyVal string) bool { return ctxVal == policyVal } -// strLike matches ctxVal against a policy pattern that may contain * wildcards. -func strLike(ctxVal, policyVal string) bool { return wildcardMatch(policyVal, ctxVal) } +// strLike matches ctxVal against a policy pattern that may contain '*' and '?' +// wildcards, case-sensitively and against the whole value. +func strLike(ctxVal, policyVal string) bool { return globMatch(policyVal, ctxVal) } // boolMatch compares the request and policy values as booleans. func boolMatch(ctxVal, policyVal string) bool { diff --git a/providers/aws/iam/iam.go b/providers/aws/iam/iam.go index a1c0e8cc4..ecf6b0adf 100644 --- a/providers/aws/iam/iam.go +++ b/providers/aws/iam/iam.go @@ -806,35 +806,6 @@ func (s *policyStatement) resourceMatches(resource string) bool { } } -func wildcardMatch(pattern, value string) bool { - if pattern == "*" { - return true - } - - pParts := strings.Split(pattern, "*") - - if len(pParts) == 1 { - return pattern == value - } - - if !strings.HasPrefix(value, pParts[0]) { - return false - } - - remaining := value[len(pParts[0]):] - - for i := 1; i < len(pParts); i++ { - idx := strings.Index(remaining, pParts[i]) - if idx < 0 { - return false - } - - remaining = remaining[idx+len(pParts[i]):] - } - - return true -} - func toStringSlice(v any) []string { switch val := v.(type) { case string: @@ -856,7 +827,7 @@ func toStringSlice(v any) []string { func matchesAction(actions []string, action string) bool { for _, a := range actions { - if wildcardMatch(a, action) { + if actionMatch(a, action) { return true } } @@ -866,7 +837,7 @@ func matchesAction(actions []string, action string) bool { func matchesResource(resources []string, resource string) bool { for _, r := range resources { - if wildcardMatch(r, resource) { + if globMatch(r, resource) { return true } } diff --git a/providers/aws/iam/iam_test.go b/providers/aws/iam/iam_test.go index 19e0e94bc..8541e82f5 100644 --- a/providers/aws/iam/iam_test.go +++ b/providers/aws/iam/iam_test.go @@ -441,7 +441,7 @@ func TestWildcardMatch(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - result := wildcardMatch(tc.pattern, tc.value) + result := globMatch(tc.pattern, tc.value) assertEqual(t, tc.expect, result) }) } diff --git a/providers/aws/iam/simulate.go b/providers/aws/iam/simulate.go index f7c25b6d1..d1a5a5765 100644 --- a/providers/aws/iam/simulate.go +++ b/providers/aws/iam/simulate.go @@ -229,11 +229,9 @@ func containsStar(resources []string) bool { } // anyCoversService reports whether one pattern matches every action of svc. -// Matching the pattern against the literal "svc:*" does that: "*", "s3:*" and -// "s*" cover s3, "s3:Get*" does not. func anyCoversService(patterns []string, svc string) bool { for _, p := range patterns { - if wildcardMatch(p, svc+":*") { + if coversService(p, svc) { return true } } @@ -241,6 +239,29 @@ func anyCoversService(patterns []string, svc string) bool { return false } +// coversService reports whether pattern matches every "svc:Action". It holds +// when the pattern is some head followed only by '*'s and the head matches a +// prefix of "svc:": the trailing stars then take the rest of any action. So "*", +// "s3:*" and "s*" cover s3 while "s3:Get*" and "s3:?" do not. It may say no for +// an odd pattern that does cover the service ("s3:?*"), never the reverse, +// which is the safe direction for both callers. +func coversService(pattern, svc string) bool { + pattern = strings.ToLower(pattern) + prefix := strings.ToLower(svc) + ":" + + for i := len(pattern) - 1; i >= 0 && pattern[i] == '*'; i-- { + head := pattern[:i] + + for k := 0; k <= len(prefix); k++ { + if globMatch(head, prefix[:k]) { + return true + } + } + } + + return false +} + func anyCouldMatchService(patterns []string, svc string) bool { for _, p := range patterns { if couldMatchService(p, svc) { @@ -255,20 +276,22 @@ func anyCouldMatchService(patterns []string, svc string) bool { // may say yes for a pattern that cannot really match, never the reverse, which // is the safe direction for both of its callers. func couldMatchService(pattern, svc string) bool { + pattern, svc = strings.ToLower(pattern), strings.ToLower(svc) + if head, _, ok := strings.Cut(pattern, ":"); ok { // Actions carry exactly one colon, so the pattern's first colon lines up // with it and the head must match the service name. - return wildcardMatch(head, svc) + return globMatch(head, svc) } - // With no colon, only a '*' can span the separator, so the text before the - // first '*' must be a prefix of the service name. - star := strings.IndexByte(pattern, '*') - if star < 0 { + // With no colon, only a wildcard can stand in for the separator, so the + // text before the first '*' or '?' must be a prefix of the service name. + wild := strings.IndexAny(pattern, "*?") + if wild < 0 { return false } - return strings.HasPrefix(svc, pattern[:star]) + return strings.HasPrefix(svc, pattern[:wild]) } // EvaluatePermission reports the tri-state decision for one action. With diff --git a/providers/aws/iam/trust_policy.go b/providers/aws/iam/trust_policy.go index e15ace989..64f97db9e 100644 --- a/providers/aws/iam/trust_policy.go +++ b/providers/aws/iam/trust_policy.go @@ -104,5 +104,5 @@ func principalEntryMatches(entry, caller string) bool { // A trust policy that names the account root trusts every principal in that // account; the caller is the account root, so an exact match already covers // it. Fall back to wildcard matching for patterns like "arn:...:role/*". - return wildcardMatch(entry, caller) + return globMatch(entry, caller) } diff --git a/providers/aws/iam/wildcard.go b/providers/aws/iam/wildcard.go new file mode 100644 index 000000000..06a661390 --- /dev/null +++ b/providers/aws/iam/wildcard.go @@ -0,0 +1,72 @@ +package iam + +import "strings" + +// arnSegments is the number of colon-separated components in an ARN: +// arn:partition:service:region:account:resource. The resource component may +// itself contain colons. +const arnSegments = 6 + +// globMatch reports whether value matches pattern under IAM's wildcard rules: +// '*' matches any run of characters (including none), '?' matches exactly one, +// and everything else is literal. The whole value must match, so the pattern is +// anchored at both ends. The comparison is case-sensitive. +func globMatch(pattern, value string) bool { + p, v := []rune(pattern), []rune(value) + pi, vi := 0, 0 + star, mark := -1, 0 + + for vi < len(v) { + switch { + case pi < len(p) && p[pi] == '*': + star, mark = pi, vi + pi++ + case pi < len(p) && (p[pi] == '?' || p[pi] == v[vi]): + pi++ + vi++ + case star >= 0: + // Let the last '*' take one more character and retry from there. + mark++ + pi, vi = star+1, mark + default: + return false + } + } + + for pi < len(p) && p[pi] == '*' { + pi++ + } + + return pi == len(p) +} + +// actionMatch matches an Action or NotAction entry against an action name. +// Action names are case-insensitive in IAM. +func actionMatch(pattern, action string) bool { + return globMatch(strings.ToLower(pattern), strings.ToLower(action)) +} + +// arnMatch implements ArnEquals and ArnLike: each of the six ARN components is +// matched on its own, so a wildcard never spans the ':' between components (the +// last component keeps any colons it carries). Matching is case-sensitive. A +// pattern with no ':' at all, such as "*", is matched against the whole value. +func arnMatch(value, pattern string) bool { + if !strings.Contains(pattern, ":") { + return globMatch(pattern, value) + } + + pp := strings.SplitN(pattern, ":", arnSegments) + vp := strings.SplitN(value, ":", arnSegments) + + if len(pp) != arnSegments || len(vp) != arnSegments { + return false + } + + for i := range pp { + if !globMatch(pp[i], vp[i]) { + return false + } + } + + return true +} diff --git a/providers/aws/iam/wildcard_match_test.go b/providers/aws/iam/wildcard_match_test.go new file mode 100644 index 000000000..c98c37ecd --- /dev/null +++ b/providers/aws/iam/wildcard_match_test.go @@ -0,0 +1,208 @@ +package iam + +import ( + "testing" +) + +// TestActionWildcardAnchored covers the Action and NotAction glob rules: '*' is +// any run (including empty), '?' is exactly one character, the pattern is +// anchored at both ends, and action names compare case-insensitively. +func TestActionWildcardAnchored(t *testing.T) { + tests := []struct { + name string + pattern string + action string + allowed bool + }{ + {"suffix star matches", "s3:*Bucket", "s3:CreateBucket", true}, + {"suffix star is anchored", "s3:*Bucket", "s3:DeleteBucketPolicy", false}, + {"suffix star anchored mid-string", "s3:*Bucket", "s3:GetBucketTagging", false}, + {"star matches empty", "s3:Get*", "s3:Get", true}, + {"star alone in middle matches empty", "s3:Get*Object", "s3:GetObject", true}, + {"question mark one char", "s3:Get?bject", "s3:GetObject", true}, + {"question mark not zero chars", "s3:Get?Object", "s3:GetObject", false}, + {"question mark not two chars", "s3:Get?bject", "s3:GetXXbject", false}, + {"exact literal", "s3:GetObject", "s3:GetObject", true}, + {"literal is anchored", "s3:GetObject", "s3:GetObjectAcl", false}, + {"action case-insensitive", "S3:getobject", "s3:GetObject", true}, + {"action wildcard case-insensitive", "dynamodb:*table", "dynamodb:DescribeTable", true}, + {"backtracking star", "s3:*Object*Acl", "s3:PutObjectVersionAcl", true}, + {"backtracking star anchored", "s3:*Object*Acl", "s3:PutObjectAclX", false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + doc := makePolicyDoc([]map[string]any{ + {"Effect": "Allow", "Action": tc.pattern, "Resource": "*"}, + }) + + want := decisionImplicitDeny + if tc.allowed { + want = decisionAllowed + } + + assertEqual(t, want, decideDoc(doc, tc.action, "*", nil)) + }) + } +} + +// TestNotActionWildcardAnchored proves NotAction uses the same anchored glob: +// an action that only contains the pattern text mid-string is not exempted. +func TestNotActionWildcardAnchored(t *testing.T) { + doc := makePolicyDoc([]map[string]any{ + {"Effect": "Allow", "Action": "s3:*", "Resource": "*"}, + {"Effect": "Deny", "NotAction": "s3:*Bucket", "Resource": "*"}, + }) + + assertEqual(t, decisionAllowed, decideDoc(doc, "s3:CreateBucket", "*", nil)) + assertEqual(t, decisionExplicitDeny, decideDoc(doc, "s3:DeleteBucketPolicy", "*", nil)) +} + +// TestResourceWildcardAnchored covers Resource and NotResource matching: same +// glob grammar as Action, but ARNs compare case-sensitively. +func TestResourceWildcardAnchored(t *testing.T) { + tests := []struct { + name string + pattern string + resource string + allowed bool + }{ + {"object under bucket", "arn:aws:s3:::b/*", "arn:aws:s3:::b/key.txt", true}, + {"nested object under bucket", "arn:aws:s3:::b/*", "arn:aws:s3:::b/dir/key.txt", true}, + {"empty key still matches star", "arn:aws:s3:::b/*", "arn:aws:s3:::b/", true}, + {"bucket itself is not an object", "arn:aws:s3:::b/*", "arn:aws:s3:::b", false}, + {"other bucket with same prefix", "arn:aws:s3:::b/*", "arn:aws:s3:::bb/key", false}, + {"literal is anchored", "arn:aws:s3:::b", "arn:aws:s3:::bucket", false}, + {"suffix pattern anchored", "arn:aws:s3:::*-logs", "arn:aws:s3:::app-logs-old", false}, + {"suffix pattern matches", "arn:aws:s3:::*-logs", "arn:aws:s3:::app-logs", true}, + {"question mark in resource", "arn:aws:s3:::b?", "arn:aws:s3:::b1", true}, + {"question mark exactly one", "arn:aws:s3:::b?", "arn:aws:s3:::b12", false}, + {"resource case-sensitive", "arn:aws:s3:::Bucket/*", "arn:aws:s3:::bucket/x", false}, + {"resource star spans colons", "arn:aws:dynamodb:*", "arn:aws:dynamodb:us-east-1:123456789012:table/t", true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + doc := makePolicyDoc([]map[string]any{ + {"Effect": "Allow", "Action": "s3:GetObject", "Resource": tc.pattern}, + }) + + want := decisionImplicitDeny + if tc.allowed { + want = decisionAllowed + } + + assertEqual(t, want, decideDoc(doc, "s3:GetObject", tc.resource, nil)) + }) + } +} + +// TestNotResourceWildcardAnchored proves NotResource exempts only the exact +// glob match. +func TestNotResourceWildcardAnchored(t *testing.T) { + doc := makePolicyDoc([]map[string]any{ + {"Effect": "Allow", "Action": "s3:*", "Resource": "*"}, + {"Effect": "Deny", "Action": "s3:*", "NotResource": "arn:aws:s3:::safe"}, + }) + + assertEqual(t, decisionAllowed, decideDoc(doc, "s3:GetObject", "arn:aws:s3:::safe", nil)) + assertEqual(t, decisionExplicitDeny, decideDoc(doc, "s3:GetObject", "arn:aws:s3:::safe-not", nil)) +} + +// TestStringLikeArnLikeGlob covers the StringLike and ArnLike condition +// operators: anchored glob with '*' and '?', case-sensitive, and ArnLike checks +// each of the six ARN components on its own. +func TestStringLikeArnLikeGlob(t *testing.T) { + const roleArn = "arn:aws:iam::123456789012:role/app" + + tests := []struct { + name string + op string + pattern string + value string + allowed bool + }{ + {"StringLike suffix anchored", "StringLike", "*-prod", "app-prod-old", false}, + {"StringLike suffix match", "StringLike", "*-prod", "app-prod", true}, + {"StringLike question mark", "StringLike", "user-?", "user-1", true}, + {"StringLike question mark exactly one", "StringLike", "user-?", "user-12", false}, + {"StringLike star matches empty", "StringLike", "home/*", "home/", true}, + {"StringLike case-sensitive", "StringLike", "Bob*", "bob", false}, + {"StringNotLike anchored", "StringNotLike", "*-prod", "app-prod-old", true}, + {"ArnLike wildcard account", "ArnLike", "arn:aws:iam::*:role/app", roleArn, true}, + {"ArnLike resource anchored", "ArnLike", "arn:aws:iam::*:role/app", roleArn + "-admin", false}, + {"ArnLike star stays in its segment", "ArnLike", "arn:aws:iam::*", roleArn, false}, + {"ArnLike star cannot swallow segments", "ArnLike", "arn:aws:*:role/app", roleArn, false}, + {"ArnLike star per segment", "ArnLike", "arn:aws:*:*:*:*", roleArn, true}, + {"ArnLike question mark in segment", "ArnLike", "arn:aws:iam::12345678901?:role/app", roleArn, true}, + {"ArnLike case-sensitive", "ArnLike", "arn:aws:iam::*:role/App", roleArn, false}, + {"ArnLike resource keeps colons", "ArnLike", "arn:aws:logs:*:*:log-group:g:*", + "arn:aws:logs:us-east-1:123456789012:log-group:g:log-stream:s", true}, + {"ArnLike value not an ARN", "ArnLike", "arn:aws:iam::*:role/app", "role/app", false}, + {"ArnEquals segment wildcard", "ArnEquals", "arn:aws:iam::*:role/app", roleArn, true}, + {"ArnNotLike anchored", "ArnNotLike", "arn:aws:iam::*:role/app", roleArn + "-admin", true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + doc := makePolicyDoc([]map[string]any{{ + "Effect": "Allow", + "Action": "s3:GetObject", + "Resource": "*", + "Condition": map[string]any{tc.op: map[string]any{"aws:PrincipalArn": tc.pattern}}, + }}) + + want := decisionImplicitDeny + if tc.allowed { + want = decisionAllowed + } + + assertEqual(t, want, decideDoc(doc, "s3:GetObject", "*", ConditionContext{"aws:PrincipalArn": tc.value})) + }) + } +} + +// TestTrustPrincipalWildcardAnchored proves a wildcard principal in a trust +// policy does not match a caller that merely starts with the pattern. +func TestTrustPrincipalWildcardAnchored(t *testing.T) { + assertEqual(t, true, principalEntryMatches("arn:aws:iam::123456789012:role/app-*", "arn:aws:iam::123456789012:role/app-x")) + assertEqual(t, false, principalEntryMatches("arn:aws:iam::123456789012:*/app", "arn:aws:iam::123456789012:role/app-admin")) + assertEqual(t, false, principalEntryMatches("arn:aws:iam::123456789012:role/*", "arn:aws:iam::123456789012:user/x")) +} + +// TestServiceWideActionMatch covers the service-wide simulation helpers, which +// reason about whole services rather than one action. +func TestServiceWideActionMatch(t *testing.T) { + covers := map[string]bool{ + "*": true, "s3:*": true, "S3:*": true, "s*": true, "s?:*": true, + "s3:Get*": false, "s3:?": false, "s3:*Bucket": false, "ec2:*": false, + } + + for p, want := range covers { + assertEqual(t, want, coversService(p, "s3")) + } + + could := map[string]bool{ + "s3:Get*": true, "S3:GetObject": true, "s?:x": true, "s3?GetObject": true, + "*Bucket": true, "ec2:*": false, "s3": false, "x?": false, + } + + for p, want := range could { + assertEqual(t, want, couldMatchService(p, "s3")) + } +} + +// TestGlobMatchEdges pins the matcher's corner cases directly. +func TestGlobMatchEdges(t *testing.T) { + assertEqual(t, true, globMatch("*", "")) + assertEqual(t, true, globMatch("**", "")) + assertEqual(t, true, globMatch("", "")) + assertEqual(t, false, globMatch("", "a")) + assertEqual(t, false, globMatch("?", "")) + assertEqual(t, true, globMatch("a*b*c", "abc")) + assertEqual(t, true, globMatch("a*b*c", "aXbYbZc")) + assertEqual(t, false, globMatch("a*b*c", "aXbYc-")) + assertEqual(t, true, globMatch("?", "é")) + assertEqual(t, true, arnMatch("anything", "*")) + assertEqual(t, false, actionMatch("s3:*Bucket", "s3:DeleteBucketPolicy")) +} From 9e62503e372be23682b85a89365706cbc11182e4 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Sun, 4 Oct 2026 16:23:23 +0530 Subject: [PATCH 2/2] fix(iam): anchor Azure/GCP policy wildcards, add conservative-mode checks --- providers/aws/iam/wildcard_match_test.go | 56 ++++++++++++++++++++++++ providers/azure/iam/iam.go | 8 +++- providers/azure/iam/iam_test.go | 3 ++ providers/gcp/iam/iam.go | 8 +++- providers/gcp/iam/iam_test.go | 3 ++ 5 files changed, 74 insertions(+), 4 deletions(-) diff --git a/providers/aws/iam/wildcard_match_test.go b/providers/aws/iam/wildcard_match_test.go index c98c37ecd..69b719bce 100644 --- a/providers/aws/iam/wildcard_match_test.go +++ b/providers/aws/iam/wildcard_match_test.go @@ -192,6 +192,62 @@ func TestServiceWideActionMatch(t *testing.T) { } } +// TestConservativeModesNeverWiden checks, over a grid of wildcard patterns, that +// the unknown-resource and service-wide answers never allow something a +// concrete evaluation denies, and never miss a Deny a concrete evaluation hits. +func TestConservativeModesNeverWiden(t *testing.T) { + patterns := []string{ + "*", "s3:*", "s3:*Bucket", "s3:Get?bject", "s?:*", "s3:?", "S3:get*", "s*", "*Bucket", "s3:*Object*", + } + actions := []string{"s3:GetObject", "s3:CreateBucket", "s3:DeleteBucketPolicy", "s3:PutObjectAcl", "s3:X"} + resources := []string{"*", "arn:aws:s3:::b", "arn:aws:s3:::b/k"} + + for _, p := range patterns { + for _, field := range []string{"Action", "NotAction"} { + allowDoc := makePolicyDoc([]map[string]any{{"Effect": "Allow", field: p, "Resource": "*"}}) + denyDoc := makePolicyDoc([]map[string]any{ + {"Effect": "Allow", "Action": "*", "Resource": "*"}, + {"Effect": "Deny", field: p, "Resource": "arn:aws:s3:::b"}, + }) + + for _, doc := range []string{allowDoc, denyDoc} { + checkConservative(t, doc, actions, resources) + } + } + } +} + +func checkConservative(t *testing.T, doc string, actions, resources []string) { + t.Helper() + + docs := []string{doc} + wide := decideWith(docs, evalRequest{service: "s3"}, evalServiceWide) + + for _, a := range actions { + unknown := decideWith(docs, evalRequest{action: a}, evalUnknownResource) + + for _, r := range resources { + known := decide(docs, a, r, nil) + + if unknown == decisionAllowed && known != decisionAllowed { + t.Errorf("unknown-resource allows %s but %s on %s is %s: %s", a, a, r, known, doc) + } + + if known == decisionExplicitDeny && unknown != decisionExplicitDeny { + t.Errorf("unknown-resource misses deny of %s on %s: %s", a, r, doc) + } + + if wide == decisionAllowed && known != decisionAllowed { + t.Errorf("service-wide allows s3 but %s on %s is %s: %s", a, r, known, doc) + } + + if known == decisionExplicitDeny && wide != decisionExplicitDeny { + t.Errorf("service-wide misses deny of %s on %s: %s", a, r, doc) + } + } + } +} + // TestGlobMatchEdges pins the matcher's corner cases directly. func TestGlobMatchEdges(t *testing.T) { assertEqual(t, true, globMatch("*", "")) diff --git a/providers/azure/iam/iam.go b/providers/azure/iam/iam.go index 642c23f5d..9ff8a11b0 100644 --- a/providers/azure/iam/iam.go +++ b/providers/azure/iam/iam.go @@ -628,7 +628,9 @@ func wildcardMatch(pattern, value string) bool { remaining := value[len(pParts[0]):] - for i := 1; i < len(pParts); i++ { + last := len(pParts) - 1 + + for i := 1; i < last; i++ { idx := strings.Index(remaining, pParts[i]) if idx < 0 { return false @@ -637,7 +639,9 @@ func wildcardMatch(pattern, value string) bool { remaining = remaining[idx+len(pParts[i]):] } - return true + // The text after the last '*' must end the value, so "a/*/read" does not + // match "a/x/readwrite". + return strings.HasSuffix(remaining, pParts[last]) } func toStringSlice(v any) []string { diff --git a/providers/azure/iam/iam_test.go b/providers/azure/iam/iam_test.go index 648109ee6..b240c4ae3 100644 --- a/providers/azure/iam/iam_test.go +++ b/providers/azure/iam/iam_test.go @@ -533,6 +533,9 @@ func TestWildcardMatch(t *testing.T) { {name: "prefix wildcard", pattern: "s3:*", value: "s3:GetObject", want: true}, {name: "suffix wildcard", pattern: "*Object", value: "s3:GetObject", want: true}, {name: "middle wildcard", pattern: "s3:*Object", value: "s3:GetObject", want: true}, + {name: "suffix is anchored", pattern: "s3:*Bucket", value: "s3:DeleteBucketPolicy", want: false}, + {name: "star matches empty", pattern: "a/*/read", value: "a//read", want: true}, + {name: "inner star anchored", pattern: "a/*/read", value: "a/x/readwrite", want: false}, } for _, tt := range tests { diff --git a/providers/gcp/iam/iam.go b/providers/gcp/iam/iam.go index 912166bad..e6b3ca0ec 100644 --- a/providers/gcp/iam/iam.go +++ b/providers/gcp/iam/iam.go @@ -649,7 +649,9 @@ func wildcardMatch(pattern, value string) bool { remaining := value[len(pParts[0]):] - for i := 1; i < len(pParts); i++ { + last := len(pParts) - 1 + + for i := 1; i < last; i++ { idx := strings.Index(remaining, pParts[i]) if idx < 0 { return false @@ -658,7 +660,9 @@ func wildcardMatch(pattern, value string) bool { remaining = remaining[idx+len(pParts[i]):] } - return true + // The text after the last '*' must end the value, so "storage.*.get" does + // not match "storage.objects.getIamPolicy". + return strings.HasSuffix(remaining, pParts[last]) } func toStringSlice(v any) []string { diff --git a/providers/gcp/iam/iam_test.go b/providers/gcp/iam/iam_test.go index e97bf6118..c729fb647 100644 --- a/providers/gcp/iam/iam_test.go +++ b/providers/gcp/iam/iam_test.go @@ -364,6 +364,9 @@ func TestWildcardMatch(t *testing.T) { {name: "prefix wildcard", pattern: "s3:*", value: "s3:GetObject", want: true}, {name: "no match", pattern: "s3:Get*", value: "s3:PutObject", want: false}, {name: "middle wildcard", pattern: "arn:aws:s3:::*/*", value: "arn:aws:s3:::bucket/key", want: true}, + {name: "suffix is anchored", pattern: "s3:*Bucket", value: "s3:DeleteBucketPolicy", want: false}, + {name: "inner star anchored", pattern: "storage.*.get", value: "storage.objects.getIamPolicy", want: false}, + {name: "inner star match", pattern: "storage.*.get", value: "storage.objects.get", want: true}, } for _, tt := range tests {