Skip to content

Commit

Permalink
fix(aws): do not flag cross-service confused deputy as public (#5593)
Browse files Browse the repository at this point in the history
  • Loading branch information
MrCloudSec authored Nov 4, 2024
1 parent 797b627 commit be523c1
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ def execute(self) -> Check_Report_AWS:
if not is_policy_public(
role.assume_role_policy,
iam_client.audited_account,
check_cross_service_confused_deputy=True,
not_allowed_actions=["sts:AssumeRole", "sts:*"],
):
report.status = "PASS"
Expand Down
25 changes: 15 additions & 10 deletions prowler/providers/aws/services/iam/lib/policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,17 @@ def is_policy_public(
source_account: str = "",
is_cross_account_allowed=True,
not_allowed_actions: list = [],
check_cross_service_confused_deputy=False,
) -> bool:
"""
Check if the policy allows public access to the resource.
If the policy gives access to an AWS service principal is considered public if the policy is not pair with conditions since it can be invoked by AWS services in other accounts.
Args:
policy (dict): The AWS policy to check
source_account (str): The account to check if the access is restricted to it, default: ""
is_cross_account_allowed (bool): If the policy can allow cross-account access, default: True
is_cross_account_allowed (bool): If the policy can allow cross-account access, default: True (https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html#cross-service-confused-deputy-prevention)
not_allowed_actions (list): List of actions that are not allowed, default: []. If not_allowed_actions is empty, the function will not consider the actions in the policy.
check_cross_service_confused_deputy (bool): If the policy is checked for cross-service confused deputy, default: False
Returns:
bool: True if the policy allows public access, False otherwise
"""
Expand Down Expand Up @@ -164,17 +166,20 @@ def is_policy_public(
or "*" in principal.get("CanonicalUser", "")
or "arn:aws:iam::*:root"
in principal.get("CanonicalUser", "")
or ( # Check if function can be invoked by other AWS services
(
".amazonaws.com" in principal.get("Service", "")
or ".amazon.com" in principal.get("Service", "")
or "*" in principal.get("Service", "")
or check_cross_service_confused_deputy
and (
( # Check if function can be invoked by other AWS services if check_cross_service_confused_deputy is True
(
".amazonaws.com" in principal.get("Service", "")
or ".amazon.com" in principal.get("Service", "")
or "*" in principal.get("Service", "")
)
)
and "secretsmanager.amazonaws.com"
not in principal.get(
"Service", ""
) # AWS ensures that resources called by SecretsManager are executed in the same AWS account
)
and "secretsmanager.amazonaws.com"
not in principal.get(
"Service", ""
) # AWS ensures that resources called by SecretsManager are executed in the same AWS account
)
)
) and (
Expand Down
38 changes: 38 additions & 0 deletions tests/providers/aws/services/iam/lib/policy_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1923,6 +1923,44 @@ def test_is_policy_public_secrets_manager(
}
assert not is_policy_public(policy, TRUSTED_AWS_ACCOUNT_NUMBER)

def test_is_policy_public_cross_cross_service_confused_deputy(
self,
):
policy = {
"Statement": [
{
"Sid": "test",
"Effect": "Allow",
"Principal": {"Service": "ec2.amazonaws.com"},
"Action": "lambda:InvokeFunction",
"Resource": "*",
}
]
}
assert is_policy_public(
policy, TRUSTED_AWS_ACCOUNT_NUMBER, check_cross_service_confused_deputy=True
)

def test_is_policy_public_cross_cross_service_confused_deputy_ignored(
self,
):
policy = {
"Statement": [
{
"Sid": "test",
"Effect": "Allow",
"Principal": {"Service": "ec2.amazonaws.com"},
"Action": "lambda:InvokeFunction",
"Resource": "*",
}
]
}
assert not is_policy_public(
policy,
TRUSTED_AWS_ACCOUNT_NUMBER,
check_cross_service_confused_deputy=False,
)

def test_is_policy_public_alexa_condition(
self,
):
Expand Down

0 comments on commit be523c1

Please sign in to comment.