Improve wildcard expressions (#221)

* Improve wildcard expressions to start considering question marks as wildcards too * Update changelog * Apply suggestions from code review Co-authored-by: Ramon <w0rmr1d3r@users.noreply.github.com> Co-authored-by: Ramon <w0rmr1d3r@users.noreply.github.com>
Skyscanner · Apr 7, 2022 · e1dea01 · e1dea01
1 parent ade94e2
commit e1dea01
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 40 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,10 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## [1.9.0]
+### Improvements
+- CFRipper is now able to detect new types of wildcard usage.
+
 ## [1.8.0]
 ### Improvements
 - Pin `click` to at least version `8.0.0`.

diff --git a/cfripper/config/regex.py b/cfripper/config/regex.py
@@ -3,11 +3,10 @@
 """
 Check for Principals where root is being used.
 Valid:
-- arn:aws:iam::437628376:root
-- arn:aws:iam::344345345:root
+- arn:aws:iam::123456789012:root
 - arn:aws:iam:::root
 Invalid:
-- arn:aws:iam::437628376:not-root
+- arn:aws:iam::123456789012:not-root
 - potato
 """
 REGEX_CROSS_ACCOUNT_ROOT = re.compile(r"arn:aws:iam::\d*:root")
@@ -16,50 +15,56 @@
 Check for use of wildcard in two bad cases: full wildcard, or wildcard in account ID.
 Valid:
 - *
+- ?*
+- **
 - arn:aws:iam::*:12345
 Invalid:
-- arn:aws:iam::444455556666:root
+- arn:aws:iam::123456789012:root
 - potato
-- arn:aws:iam::12345:*
+- arn:aws:iam::123456789012:*
 """
-REGEX_FULL_WILDCARD_PRINCIPAL = re.compile(r"^((\w*:){0,1}\*|arn:aws:iam::\*:.*)$")
+REGEX_FULL_WILDCARD_PRINCIPAL = re.compile(r"^((\w*:)?[*?]+|arn:aws:iam::[*?]+:.*)$")
 
 """
 Check for use of wildcard or account-wide principals.
 Valid:
 - arn:aws:iam::123456789012:*
 - arn:aws:iam::123456789012:service-*
 - arn:aws:iam::123456789012:root
+- arn:aws:iam::123456789012:*-role
 - 123456789012
 Invalid:
 - *
 - potato
-- arn:aws:iam::123456789012:*not-root
+- arn:aws:iam::123456789012:not-root
 """
-REGEX_PARTIAL_WILDCARD_PRINCIPAL = re.compile(r"^\d{12}|arn:aws:iam::.*:(.*\*|root)$")
+REGEX_PARTIAL_WILDCARD_PRINCIPAL = re.compile(r"^(\d+)|(arn:aws:iam::(\d+):(.*[*?]+|[*?]+.*|root))$")
 
 """
 Check for use of wildcard, when applied to the specific elements of an Action.
 For example, sts:AssumeRole* or sts:*. This regex is not checking for use of `*` on its own.
 Valid:
 - sts:AssumeRole*
 - sts:*
+- sts:Assume????
+- sts:??????Role
+- sts:*Role*
 Invalid:
 - sts:AssumeRole
 - sts:AssumeRole-Thing-This
 - *
 """
-REGEX_WILDCARD_POLICY_ACTION = re.compile(r"^(\w*:)(\w*)\*(\w*)$")
+REGEX_WILDCARD_POLICY_ACTION = re.compile(r"^(\w*:)(.*)[*?]+(.*)$")
 
 """
-Check for Principals where root is being used.
+Check for Principals where a star is being used.
 Valid:
 - *
 - abc*def
 - abcdef*
 - *abcdef
 Invalid:
-- arn:aws:iam::437628376:not-root
+- arn:aws:iam::123456789012:not-root
 - potato
 """
 REGEX_CONTAINS_STAR = re.compile(r"^.*[*].*$")
@@ -73,7 +78,7 @@
 - abc*def
 - abcdef*
 - *abcdef
-- arn:aws:iam::437628376:not-root
+- arn:aws:iam::123456789012:not-root
 - potato
 """
 REGEX_IS_STAR = re.compile(r"^\*$")
@@ -84,8 +89,8 @@
 It has 4 groups. The first one for service name, the second one for region, the third, for account id, the last one
 for resource id
 Valid:
-- arn:aws:iam::437628376:not-root
-- arn:aws:iam::437628376:root
+- arn:aws:iam::123456789012:not-root
+- arn:aws:iam::123456789012:root
 - arn:aws:s3:::my_corporate_bucket
 Invalid:
 - potato
@@ -102,40 +107,42 @@
 Invalid:
 - potato
 - arn:aws:s3:::my_corporate_bucket
-- arn:aws:iam::437628376:root
+- arn:aws:iam::123456789012:root
 """
-REGEX_WILDCARD_ARN = re.compile(r"^arn:aws:([*\w]+):(\*)?:(\*)?:([*?]+)$")
+REGEX_WILDCARD_ARN = re.compile(r"^arn:aws:([*\w]+):([*?]*):([*?]*):([*?]+)$")
 
 
 """
 Check for iam arns
 It has 2 groups. The first one for account id, the last one for resource id
 Valid:
-- arn:aws:iam::437628376:not-root
+- arn:aws:iam::123456789012:not-root
 Invalid:
 - arn:aws:s3:::my_corporate_bucket
 - potato
 """
-REGEX_IAM_ARN = re.compile(r"^arn:aws:iam::(\d*):(.*)$")
+REGEX_IAM_ARN = re.compile(r"^arn:aws:iam::(\d+):(.*)$")
 
 
 """
 Check for sts arns
 It has 2 groups. The first one for account id, the last one for resource id
 Valid:
-- arn:aws:sts::437628376:not-root
+- arn:aws:sts::123456789012:not-root
 Invalid:
 - arn:aws:s3:::my_corporate_bucket
 - potato
 """
-REGEX_STS_ARN = re.compile(r"^arn:aws:sts::(\d*):(.*)$")
+REGEX_STS_ARN = re.compile(r"^arn:aws:sts::(\d+):(.*)$")
 
 
 """
-Check for wildcards after colons
+Check for a wildcard star or wildcards immediately after the last colon
 Valid:
 - *
-- arn:aws:iam::437628376:*
+- arn:aws:iam::123456789012:*
+- arn:aws:iam::123456789012:??????
+- arn:aws:iam::123456789012:?*
 - sns:*
 Invalid:
 - arn:aws:s3:::my_corporate_bucket
@@ -144,4 +151,4 @@
 - potato
 - sns:Get*
 """
-REGEX_HAS_STAR_OR_STAR_AFTER_COLON = re.compile(r"^(\w*:)*\*$")
+REGEX_HAS_STAR_OR_STAR_AFTER_COLON = re.compile(r"^(\w*:)*[*?]+$")
diff --git a/tests/config/test_regex.py b/tests/config/test_regex.py
@@ -18,54 +18,61 @@
 @pytest.mark.parametrize(
     "regex, data, valid",
     [
-        (REGEX_CROSS_ACCOUNT_ROOT, "arn:aws:iam::437628376:root", True),
-        (REGEX_CROSS_ACCOUNT_ROOT, "arn:aws:iam::344345345:root", True),
+        (REGEX_CROSS_ACCOUNT_ROOT, "arn:aws:iam::123456789012:root", True),
         (REGEX_CROSS_ACCOUNT_ROOT, "arn:aws:iam:::root", True),
-        (REGEX_CROSS_ACCOUNT_ROOT, "arn:aws:iam::437628376:not-root", False),
+        (REGEX_CROSS_ACCOUNT_ROOT, "arn:aws:iam::123456789012:not-root", False),
         (REGEX_CROSS_ACCOUNT_ROOT, "potato", False),
         (REGEX_FULL_WILDCARD_PRINCIPAL, "*", True),
+        (REGEX_FULL_WILDCARD_PRINCIPAL, "**", True),
+        (REGEX_FULL_WILDCARD_PRINCIPAL, "*?", True),
+        (REGEX_FULL_WILDCARD_PRINCIPAL, "*???????*", True),
         (REGEX_FULL_WILDCARD_PRINCIPAL, "arn:aws:iam::*:12345", True),
         (REGEX_FULL_WILDCARD_PRINCIPAL, "arn:aws:iam::444455556666:root", False),
         (REGEX_FULL_WILDCARD_PRINCIPAL, "potato", False),
-        (REGEX_FULL_WILDCARD_PRINCIPAL, "arn:aws:iam::12345:*", False),
+        (REGEX_FULL_WILDCARD_PRINCIPAL, "arn:aws:iam::123456789012:*", False),
         (REGEX_WILDCARD_POLICY_ACTION, "sts:AssumeRole*", True),
         (REGEX_WILDCARD_POLICY_ACTION, "sts:*", True),
+        (REGEX_WILDCARD_POLICY_ACTION, "sts:Assume????", True),
+        (REGEX_WILDCARD_POLICY_ACTION, "sts:??????Role", True),
+        (REGEX_WILDCARD_POLICY_ACTION, "sts:*Role*", True),
         (REGEX_WILDCARD_POLICY_ACTION, "sts:AssumeRole", False),
         (REGEX_WILDCARD_POLICY_ACTION, "sts:AssumeRole-Thing-This", False),
         (REGEX_WILDCARD_POLICY_ACTION, "*", False),
         (REGEX_CONTAINS_STAR, "*", True),
         (REGEX_CONTAINS_STAR, "abc*def", True),
         (REGEX_CONTAINS_STAR, "abcdef*", True),
         (REGEX_CONTAINS_STAR, "*abcdef", True),
-        (REGEX_CONTAINS_STAR, "arn:aws:iam::437628376:not-root", False),
+        (REGEX_CONTAINS_STAR, "arn:aws:iam::123456789012:not-root", False),
         (REGEX_CONTAINS_STAR, "potato", False),
         (REGEX_IS_STAR, "*", True),
         (REGEX_IS_STAR, "abc*def", False),
         (REGEX_IS_STAR, "abcdef*", False),
         (REGEX_IS_STAR, "*abcdef", False),
-        (REGEX_IS_STAR, "arn:aws:iam::437628376:not-root", False),
+        (REGEX_IS_STAR, "arn:aws:iam::123456789012:not-root", False),
         (REGEX_IS_STAR, "potato", False),
-        (REGEX_ARN, "arn:aws:iam::437628376:not-root", True),
-        (REGEX_ARN, "arn:aws:iam::437628376:root", True),
+        (REGEX_ARN, "arn:aws:iam::123456789012:not-root", True),
+        (REGEX_ARN, "arn:aws:iam::123456789012:root", True),
         (REGEX_ARN, "arn:aws:s3:::my_corporate_bucket", True),
         (REGEX_ARN, "potato", False),
         (REGEX_WILDCARD_ARN, "arn:aws:s3:::*", True),
         (REGEX_WILDCARD_ARN, "arn:aws:iam:*:*:*", True),
         (REGEX_WILDCARD_ARN, "arn:aws:*:::*", True),
         (REGEX_WILDCARD_ARN, "arn:aws:s3:::my_corporate_bucket", False),
-        (REGEX_WILDCARD_ARN, "arn:aws:iam::437628376:root", False),
-        (REGEX_IAM_ARN, "arn:aws:iam::437628376:not-root", True),
-        (REGEX_IAM_ARN, "arn:aws:iam::437628376:root", True),
+        (REGEX_WILDCARD_ARN, "arn:aws:iam::123456789012:root", False),
+        (REGEX_IAM_ARN, "arn:aws:iam::123456789012:not-root", True),
+        (REGEX_IAM_ARN, "arn:aws:iam::123456789012:root", True),
         (REGEX_IAM_ARN, "arn:aws:s3:::my_corporate_bucket", False),
         (REGEX_IAM_ARN, "potato", False),
-        (REGEX_STS_ARN, "arn:aws:sts::437628376:not-root", True),
-        (REGEX_STS_ARN, "arn:aws:sts::437628376:root", True),
+        (REGEX_STS_ARN, "arn:aws:sts::123456789012:not-root", True),
+        (REGEX_STS_ARN, "arn:aws:sts::123456789012:root", True),
         (REGEX_STS_ARN, "arn:aws:s3:::my_corporate_bucket", False),
         (REGEX_STS_ARN, "potato", False),
-        (REGEX_HAS_STAR_OR_STAR_AFTER_COLON, "arn:aws:iam::437628376:*", True),
         (REGEX_HAS_STAR_OR_STAR_AFTER_COLON, "*", True),
+        (REGEX_HAS_STAR_OR_STAR_AFTER_COLON, "arn:aws:iam::123456789012:*", True),
+        (REGEX_HAS_STAR_OR_STAR_AFTER_COLON, "arn:aws:iam::123456789012:??????", True),
+        (REGEX_HAS_STAR_OR_STAR_AFTER_COLON, "arn:aws:iam::123456789012:?*", True),
         (REGEX_HAS_STAR_OR_STAR_AFTER_COLON, "sns:*", True),
-        (REGEX_HAS_STAR_OR_STAR_AFTER_COLON, "arn:aws:iam::437628376:root", False),
+        (REGEX_HAS_STAR_OR_STAR_AFTER_COLON, "arn:aws:iam::123456789012:root", False),
         (REGEX_HAS_STAR_OR_STAR_AFTER_COLON, "arn:aws:s3:::my_corporate_bucket*", False),
         (REGEX_HAS_STAR_OR_STAR_AFTER_COLON, "arn:aws:s3:::*my_corporate_bucket", False),
         (REGEX_HAS_STAR_OR_STAR_AFTER_COLON, "potato", False),
@@ -76,7 +83,7 @@
         (REGEX_PARTIAL_WILDCARD_PRINCIPAL, "123456789012", True),
         (REGEX_PARTIAL_WILDCARD_PRINCIPAL, "*", False),
         (REGEX_PARTIAL_WILDCARD_PRINCIPAL, "potato", False),
-        (REGEX_PARTIAL_WILDCARD_PRINCIPAL, "arn:aws:iam::123456789012:*not-root", False),
+        (REGEX_PARTIAL_WILDCARD_PRINCIPAL, "arn:aws:iam::123456789012:*-role", True),
     ],
 )
 def test_regex_cross_account_root(regex, data, valid):

diff --git a/tests/conftest.py b/tests/conftest.py
@@ -31,7 +31,7 @@ def test_files_location() -> Path:
 def default_allow_all_config():
     return Config(
         rules=DEFAULT_RULES,
-        aws_account_id="123456789",
+        aws_account_id="123456789012",
         stack_name="mockstack",
         rules_filters=[
             Filter(