fix(aws): make is_service_role more restrictive to avoid false positives (#8274)

Co-authored-by: MrCloudSec <hello@mistercloudsec.com>
This commit is contained in:
Kay Agahd
2025-07-15 16:02:09 +02:00
committed by GitHub
parent c82cd5288c
commit bf0013dae3
3 changed files with 44 additions and 28 deletions
+1
View File
@@ -18,6 +18,7 @@ All notable changes to the **Prowler SDK** are documented in this file.
### Fixed
- Title & description wording for `iam_user_accesskey_unused` check for AWS provider [(#8233)](https://github.com/prowler-cloud/prowler/pull/8233)
- Add GitHub provider to lateral panel in documentation and change -h environment variable output [(#8246)](https://github.com/prowler-cloud/prowler/pull/8246)
- Ensure `is_service_role` only returns `True` for service roles [(#8274)](https://github.com/prowler-cloud/prowler/pull/8274)
- Update DynamoDB check metadata to fix broken link [(#8273)](https://github.com/prowler-cloud/prowler/pull/8273)
- Show correct count of findings in Dashboard Security Posture page [(#8270)](https://github.com/prowler-cloud/prowler/pull/8270)
@@ -13,38 +13,28 @@ from prowler.providers.aws.lib.service.service import AWSService
def is_service_role(role):
try:
if "Statement" in role["AssumeRolePolicyDocument"]:
if isinstance(role["AssumeRolePolicyDocument"]["Statement"], list):
for statement in role["AssumeRolePolicyDocument"]["Statement"]:
if (
statement["Effect"] == "Allow"
and (
"sts:AssumeRole" in statement["Action"]
or "sts:*" in statement["Action"]
or "*" in statement["Action"]
)
# This is what defines a service role
and "Service" in statement["Principal"]
):
return True
else:
statement = role["AssumeRolePolicyDocument"]["Statement"]
if (
statement["Effect"] == "Allow"
and (
"sts:AssumeRole" in statement["Action"]
or "sts:*" in statement["Action"]
or "*" in statement["Action"]
)
# This is what defines a service role
and "Service" in statement["Principal"]
):
return True
statements = role.get("AssumeRolePolicyDocument", {}).get("Statement", [])
if not isinstance(statements, list):
statements = [statements]
for statement in statements:
if statement.get("Effect") != "Allow" or not any(
action in statement.get("Action", [])
for action in ("sts:AssumeRole", "sts:*", "*")
):
return False
principal = statement.get("Principal", {})
if set(principal.keys()) != {"Service"}:
return False
return True
except Exception as error:
logger.error(
f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
)
return False
return False
class IAM(AWSService):
@@ -286,6 +286,22 @@ class Test_IAM_Service:
}
],
}
# Hybrid role - assumable by both service and AWS account
hybrid_policy_document = {
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Principal": {"Service": "cloudformation.amazonaws.com"},
"Action": "sts:AssumeRole",
},
{
"Effect": "Allow",
"Principal": {"AWS": "arn:aws:iam::123456789012:root"},
"Action": "sts:AssumeRole",
},
],
}
service_role = iam_client.create_role(
RoleName="test-1",
AssumeRolePolicyDocument=dumps(service_policy_document),
@@ -300,6 +316,13 @@ class Test_IAM_Service:
{"Key": "test", "Value": "test"},
],
)["Role"]
hybrid_role = iam_client.create_role(
RoleName="test-3",
AssumeRolePolicyDocument=dumps(hybrid_policy_document),
Tags=[
{"Key": "test", "Value": "test"},
],
)["Role"]
# IAM client for this test class
aws_provider = set_mocked_aws_provider([AWS_REGION_US_EAST_1])
@@ -314,6 +337,8 @@ class Test_IAM_Service:
]
assert is_service_role(service_role)
assert not is_service_role(role)
# Hybrid role should return False even though it has a service principal
assert not is_service_role(hybrid_role)
# Test IAM Get Groups
@mock_aws