From 3c22a740189f1f95dba8b3969461c01cff5d5654 Mon Sep 17 00:00:00 2001
From: Kevin DeJong <kddejong@amazon.com>
Date: Mon, 15 Apr 2024 14:05:10 -0700
Subject: [PATCH] Check for unsupported element combination

---
 src/cfnlint/rules/resources/iam/Policy.py     | 12 ++++++++++++
 .../bad/resources/iam/iam_policy.yaml         | 19 +++++++++++++++++++
 .../bad/resources/iam/resource_policy.yaml    | 14 ++++++++++++++
 .../rules/resources/iam/test_iam_policy.py    |  4 ++--
 4 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/src/cfnlint/rules/resources/iam/Policy.py b/src/cfnlint/rules/resources/iam/Policy.py
index e6848dcfbc..09a87ac068 100644
--- a/src/cfnlint/rules/resources/iam/Policy.py
+++ b/src/cfnlint/rules/resources/iam/Policy.py
@@ -151,6 +151,10 @@ def _check_policy_statement(
         if "Action" not in statement and "NotAction" not in statement:
             message = "IAM Policy statement missing Action or NotAction"
             matches.append(RuleMatch(branch[:], message))
+        if "Action" in statement and "NotAction" in statement:
+            message = "IAM Policy statement should have only one of Action or NotAction"
+            matches.append(RuleMatch(branch[:] + ["Action"], message))
+            matches.append(RuleMatch(branch[:] + ["NotAction"], message))
         if is_identity_policy:
             if "Principal" in statement or "NotPrincipal" in statement:
                 message = "IAM Resource Policy statement shouldn't have Principal or NotPrincipal"
@@ -159,10 +163,18 @@ def _check_policy_statement(
             if "Principal" not in statement and "NotPrincipal" not in statement:
                 message = "IAM Resource Policy statement should have Principal or NotPrincipal"
                 matches.append(RuleMatch(branch[:] + ["Principal"], message))
+            if "Principal" in statement and "NotPrincipal" in statement:
+                message = "IAM Resource Policy statement should have only one of Principal or NotPrincipal"
+                matches.append(RuleMatch(branch[:] + ["Principal"], message))
+                matches.append(RuleMatch(branch[:] + ["NotPrincipal"], message))
         if not resource_exceptions:
             if "Resource" not in statement and "NotResource" not in statement:
                 message = "IAM Policy statement missing Resource or NotResource"
                 matches.append(RuleMatch(branch[:], message))
+            if "Resource" in statement and "NotResource" in statement:
+                message = "IAM Resource Policy statement should have only one of Resource or NotResource"
+                matches.append(RuleMatch(branch[:] + ["Resource"], message))
+                matches.append(RuleMatch(branch[:] + ["NotResource"], message))
 
         resources = statement.get("Resource", [])
         if isinstance(resources, str):
diff --git a/test/fixtures/templates/bad/resources/iam/iam_policy.yaml b/test/fixtures/templates/bad/resources/iam/iam_policy.yaml
index d55662dfa5..8d6db2fdc3 100644
--- a/test/fixtures/templates/bad/resources/iam/iam_policy.yaml
+++ b/test/fixtures/templates/bad/resources/iam/iam_policy.yaml
@@ -40,3 +40,22 @@ Resources:
         - cCondition
         - Statement: {}
         - Statement: []
+  rIamPolicyExtraThings:
+    Type: AWS::IAM::Policy
+    Properties:
+      PolicyName: test
+      Roles:
+      - MyRole
+      PolicyDocument:
+        Version: '2012-10-17'
+        Statement:
+        - Effect: Allow
+
+          Action:
+          - cloudwatch:*
+          NotAction:
+          - ec2:*
+          Resource:
+          - '*'
+          NotResource:
+          - 'arn'
diff --git a/test/fixtures/templates/bad/resources/iam/resource_policy.yaml b/test/fixtures/templates/bad/resources/iam/resource_policy.yaml
index 5e777c7c4c..1686e68b17 100644
--- a/test/fixtures/templates/bad/resources/iam/resource_policy.yaml
+++ b/test/fixtures/templates/bad/resources/iam/resource_policy.yaml
@@ -16,6 +16,20 @@ Resources:
                 aws:Referer:
                   - "http://www.example.com/*"
                   - "http://example.com/*"
+  SampleBucketPolicy2:
+    Type: AWS::S3::BucketPolicy
+    Properties:
+      Bucket: "myExampleBucket"
+      PolicyDocument:
+        Statement:
+          -
+            Action:
+              - "s3:GetObject"
+            Effect: "Allow"
+            Resource:
+            - "*"
+            Principal: test
+            NotPrincipal: test
   mysnspolicy:
     Type: AWS::SNS::TopicPolicy
     Properties:
diff --git a/test/unit/rules/resources/iam/test_iam_policy.py b/test/unit/rules/resources/iam/test_iam_policy.py
index 3bcf9145a2..fa11f9e4c8 100644
--- a/test/unit/rules/resources/iam/test_iam_policy.py
+++ b/test/unit/rules/resources/iam/test_iam_policy.py
@@ -27,11 +27,11 @@ def test_file_positive(self):
     def test_file_negative(self):
         """Test failure"""
         self.helper_file_negative(
-            "test/fixtures/templates/bad/resources/iam/iam_policy.yaml", 12
+            "test/fixtures/templates/bad/resources/iam/iam_policy.yaml", 16
         )
 
     def test_file_resource_negative(self):
         """Test failure"""
         self.helper_file_negative(
-            "test/fixtures/templates/bad/resources/iam/resource_policy.yaml", 4
+            "test/fixtures/templates/bad/resources/iam/resource_policy.yaml", 6
         )