Merge pull request #24 from kmcquade/fix/GH-23-bool-object-not-iterable

Fixes #23 - issue arising from policies with "Deny" with no resource constraints

Author: kmcquade

CHANGELOG.md

@@ -1,5 +1,8 @@
 # CHANGELOG
 
+## 0.0.11 (2020-05-06)
+* Fixed an issue arising from policies where "Deny" was used in effect with no resource constraints. Fixes #23.
+
 ## 0.0.10 (2020-05-05)
 * Removed the recursive credentials method from the `download` command.
 * Fixed occasional installation error occurring from outdated Policy Sentry versions.

cloudsplaining/bin/cloudsplaining

@@ -7,7 +7,7 @@
 """
     Cloudsplaining is an AWS IAM Assessment tool that identifies violations of least privilege and generates a risk-prioritized HTML report with a triage worksheet.
 """
-__version__ = "0.0.10"
+__version__ = "0.0.11"
 import click
 from cloudsplaining import command
 

cloudsplaining/scan/policy_document.py

@@ -40,7 +40,8 @@ def all_allowed_actions(self):
         """Output all allowed IAM Actions, regardless of resource constraints"""
         allowed_actions = []
         for statement in self.statements:
-            allowed_actions.extend(statement.expanded_actions)
+            if statement.expanded_actions:
+                allowed_actions.extend(statement.expanded_actions)
         allowed_actions = list(dict.fromkeys(allowed_actions))
         return allowed_actions
 
@@ -50,7 +51,8 @@ def all_allowed_unrestricted_actions(self):
         allowed_actions = []
         for statement in self.statements:
             if not statement.has_resource_constraints:
-                allowed_actions.extend(statement.expanded_actions)
+                if statement.expanded_actions:
+                    allowed_actions.extend(statement.expanded_actions)
         allowed_actions = list(dict.fromkeys(allowed_actions))
         return allowed_actions
 

cloudsplaining/scan/statement_detail.py

@@ -81,7 +81,7 @@ def _not_action_effective_actions(self):
         """If NotAction is used, calculate the allowed actions - i.e., what it would be """
         effective_actions = []
         if not self.not_action:
-            return False
+            return None
         not_actions_expanded = determine_actions_to_expand(self.not_action)
         not_actions_expanded_lowercase = [x.lower() for x in not_actions_expanded]
 
@@ -116,12 +116,12 @@ def _not_action_effective_actions(self):
             return effective_actions
         elif self.has_resource_constraints and self.effect_deny:
             logger.debug("NOTE: Haven't decided if we support Effect Deny here?")
-            return False
+            return None
         elif not self.has_resource_constraints and self.effect_deny:
             logger.debug("NOTE: Haven't decided if we support Effect Deny here?")
-            return False
+            return None
         else:
-            return False
+            return None
 
     @property
     def has_not_resource_with_allow(self):

test/scanning/test_policy_document.py

@@ -182,3 +182,23 @@ def test_allows_specific_actions(self):
         ]
         results = policy_document.allows_specific_actions_without_constraints(high_priority_read_only_actions)
         self.assertListEqual(results, high_priority_read_only_actions)
+
+    def test_policy_document_not_action_deny_gh_23(self):
+        test_policy = {
+            "Version": "2012-10-17",
+            "Statement": [{
+                "Sid": "DenyAllUsersNotUsingMFA",
+                "Effect": "Deny",
+                "NotAction": "iam:*",
+                "Resource": "*",
+                "Condition": {"BoolIfExists": {"aws:MultiFactorAuthPresent": "false"}}
+            }]
+        }
+        policy_document = PolicyDocument(test_policy)
+        allowed_actions = []
+        for statement in policy_document.statements:
+            if not statement.has_resource_constraints:
+                if statement.expanded_actions:
+                    allowed_actions.extend(statement.expanded_actions)
+        self.assertListEqual(allowed_actions, [])
+        self.assertListEqual(policy_document.all_allowed_unrestricted_actions, [])