Repository: Netflix-Skunkworks/policyuniverse

Issue:
Unhandled exception ValueError when Token not found in statement._userid_internet_accessible()
https://github.com/Netflix-Skunkworks/policyuniverse/blob/96336a5c51f0daa5cc832050b5668cd9a326e6cc/policyuniverse/statement.py#L325

_userid_internet_accessible uses string.index() which throws an ValueError if the token is not present. This should be changed to string.find() which returns -1 if the token is not found.

*Reproduce*
Have an S3 bucket policy with UserId value that does not contain a "*"


Discussion:

Patch:
diff --git a/policyuniverse/__init__.py b/policyuniverse/__init__.py
index f442d13..e63a96b 100644
--- a/policyuniverse/__init__.py
+++ b/policyuniverse/__init__.py
@@ -13,7 +13,6 @@ from policyuniverse.action import build_service_actions_from_service_data
 
 
 # Logging
-logging.basicConfig(level=logging.INFO)
 logger = logging.getLogger(__name__)
 
 # Read Input Data
diff --git a/policyuniverse/arn.py b/policyuniverse/arn.py
index 61a4e97..9aa07d6 100644
--- a/policyuniverse/arn.py
+++ b/policyuniverse/arn.py
@@ -35,46 +35,46 @@ class ARN(object):
     root = False
     service = False
 
-    def __init__(self, input):
-        self.arn = input
+    def __init__(self, raw):
+        self.arn = raw
         arn_match = re.search(
-            r"^arn:([^:]*):([^:]*):([^:]*):(|\*|[\d]{12}|cloudfront|aws):(.+)$", input
+            r"^arn:([^:]*):([^:]*):([^:]*):(|\*|[\d]{12}|cloudfront|aws):(.+)$", raw
         )
         if arn_match:
             if arn_match.group(2) == "iam" and arn_match.group(5) == "root":
                 self.root = True
 
-            self._from_arn(arn_match, input)
+            self._from_arn(arn_match)
             return
 
-        acct_number_match = re.search(r"^(\d{12})+$", input)
+        acct_number_match = re.search(r"^(\d{12})+$", raw)
         if acct_number_match:
-            self._from_account_number(input)
+            self._from_account_number(raw)
             return
 
-        aws_service_match = re.search(r"^(([^.]+)(.[^.]+)?)\.amazon(aws)?\.com$", input)
+        aws_service_match = re.search(r"^(([^.]+)(.[^.]+)?)\.amazon(aws)?\.com$", raw)
         if aws_service_match:
-            self._from_aws_service(input, aws_service_match.group(1))
+            self._from_aws_service(aws_service_match.group(1))
             return
 
-        aws_service_match = re.search(r"^([^.]+).aws.internal$", input)
+        aws_service_match = re.search(r"^([^.]+).aws.internal$", raw)
         if aws_service_match:
-            self._from_aws_service(input, aws_service_match.group(1))
+            self._from_aws_service(aws_service_match.group(1))
             return
 
         self.error = True
-        logger.warning("ARN Could not parse [{}].".format(input))
+        logger.debug("ARN Could not parse [{}].".format(raw))
 
-    def _from_arn(self, arn_match, input):
+    def _from_arn(self, arn_match):
         self.partition = arn_match.group(1)
         self.tech = arn_match.group(2)
         self.region = arn_match.group(3)
         self.account_number = arn_match.group(4)
         self.name = arn_match.group(5)
 
-    def _from_account_number(self, input):
-        self.account_number = input
+    def _from_account_number(self, raw):
+        self.account_number = raw
 
-    def _from_aws_service(self, input, service):
+    def _from_aws_service(self, service):
         self.tech = service
         self.service = True
diff --git a/policyuniverse/organization.py b/policyuniverse/organization.py
index de96c9e..ea625ab 100644
--- a/policyuniverse/organization.py
+++ b/policyuniverse/organization.py
@@ -52,19 +52,19 @@ class Organization(object):
             self.organization = orgid
         else:
             self.error = True
-            logger.warning("Organization Org ID parse error [{}].".format(input))
+            logger.debug("Organization Org ID parse error [{}].".format(input))
 
     def _parse_root(self, root):
         if root.startswith("r-") or root == "*":
             self.root = root
         else:
             self.error = True
-            logger.warning("Organization root parse error [{}].".format(input))
+            logger.debug("Organization root parse error [{}].".format(input))
 
     def _parse_ou_path(self, ou):
         if self.valid_for_parent_ou or self.valid_for_child_ous:
             self.error = True
-            logger.warning("Organization OU validity error [{}].".format(input))
+            logger.debug("Organization OU validity error [{}].".format(input))
             return
 
         if not ou:
@@ -81,4 +81,4 @@ class Organization(object):
                 self.ou_path.append(ou)
             else:
                 self.error = True
-                logger.warning("Organization OU parse error [{}].".format(input))
+                logger.debug("Organization OU parse error [{}].".format(input))
diff --git a/policyuniverse/statement.py b/policyuniverse/statement.py
index be8430c..f456a36 100644
--- a/policyuniverse/statement.py
+++ b/policyuniverse/statement.py
@@ -322,7 +322,8 @@ class Statement(object):
     def _userid_internet_accessible(self, userid):
         # Trailing wildcards are okay for userids:
         # AROAIIIIIIIIIIIIIIIII:*
-        if userid.index("*") == len(userid) - 1:
+        if userid.find("*") == len(userid) - 1:
+            # note: this will also return False for a zero-length userid
             return False
         return True
 
@@ -332,7 +333,7 @@ class Statement(object):
 
         arn = ARN(arn_input)
         if arn.error:
-            logger.warning("Auditor could not parse ARN {arn}.".format(arn=arn_input))
+            logger.debug("Auditor could not parse ARN {arn}.".format(arn=arn_input))
             return "*" in arn_input
 
         if arn.tech == "s3":
@@ -340,7 +341,7 @@ class Statement(object):
             return False
 
         if not arn.account_number and not arn.service:
-            logger.warning(
+            logger.debug(
                 "Auditor could not parse Account Number from ARN {arn}.".format(
                     arn=arn_input
                 )
@@ -355,7 +356,7 @@ class Statement(object):
     def _organization_internet_accessible(self, org_input):
         organization = Organization(org_input)
         if organization.error:
-            logger.warning("Auditor could not parse Org {org}.".format(org=org_input))
+            logger.debug("Auditor could not parse Org {org}.".format(org=org_input))
             return "o-*" in org_input
 
         if organization.organization == "o-*":


Test patch:
diff --git a/policyuniverse/tests/__init__.py b/policyuniverse/tests/__init__.py
index e69de29..ca0e42e 100644
--- a/policyuniverse/tests/__init__.py
+++ b/policyuniverse/tests/__init__.py
@@ -0,0 +1,3 @@
+import logging
+
+logging.basicConfig(level=logging.INFO)
diff --git a/policyuniverse/tests/test_statement.py b/policyuniverse/tests/test_statement.py
index b55ecb4..cda547b 100644
--- a/policyuniverse/tests/test_statement.py
+++ b/policyuniverse/tests/test_statement.py
@@ -396,6 +396,15 @@ statement35 = dict(
     },
 )
 
+# AWS:userid with no *
+statement36 = dict(
+    Effect="Allow",
+    Principal="*",
+    Action=["rds:*"],
+    Resource="*",
+    Condition={"StringLike": {"AWS:userid": "AROAI1111111111111111:"}},
+)
+
 
 class StatementTestCase(unittest.TestCase):
     def test_statement_effect(self):
@@ -596,3 +605,6 @@ class StatementTestCase(unittest.TestCase):
 
         # AWS:PrincipalOrgPath Wildcard
         self.assertTrue(Statement(statement35).is_internet_accessible())
+
+        # AWS:userid with no *
+        self.assertTrue(Statement(statement36).is_internet_accessible())