From bf0013dae30049c0f2d990f16984c24e87e4a15b Mon Sep 17 00:00:00 2001 From: Kay Agahd Date: Tue, 15 Jul 2025 16:02:09 +0200 Subject: [PATCH] fix(aws): make `is_service_role` more restrictive to avoid false positives (#8274) Co-authored-by: MrCloudSec --- prowler/CHANGELOG.md | 1 + .../providers/aws/services/iam/iam_service.py | 46 ++++++++----------- .../aws/services/iam/iam_service_test.py | 25 ++++++++++ 3 files changed, 44 insertions(+), 28 deletions(-) diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index 828b4cea6a..27dc3968b2 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -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) diff --git a/prowler/providers/aws/services/iam/iam_service.py b/prowler/providers/aws/services/iam/iam_service.py index 27af7b8019..d56f6e446b 100644 --- a/prowler/providers/aws/services/iam/iam_service.py +++ b/prowler/providers/aws/services/iam/iam_service.py @@ -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): diff --git a/tests/providers/aws/services/iam/iam_service_test.py b/tests/providers/aws/services/iam/iam_service_test.py index 615d25dec6..70835f3d69 100644 --- a/tests/providers/aws/services/iam/iam_service_test.py +++ b/tests/providers/aws/services/iam/iam_service_test.py @@ -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