Skip to content

Commit

Permalink
s3 bucket confused deputy attack (#1416)
Browse files Browse the repository at this point in the history
Co-authored-by: bcpenta <bpenta@Bharats-Mac-mini.local>
Co-authored-by: Ariel Ropek <79653153+arielkr256@users.noreply.github.com>
Co-authored-by: Ariel <ariel.ropek@panther.com>
  • Loading branch information
4 people authored Nov 12, 2024
1 parent 7afcef7 commit 6224265
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 0 deletions.
1 change: 1 addition & 0 deletions packs/aws.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ PackDefinition:
- AWS.S3.Bucket.PublicWrite
- AWS.S3.Bucket.SecureAccess
- AWS.S3.Bucket.Versioning
- AWS.S3.Bucket.PolicyConfusedDeputyProtection
# Encryption Status
- AWS.EC2.EBS.Encryption.Disabled
- AWS.EC2.Volume.Encryption
Expand Down
30 changes: 30 additions & 0 deletions policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import json

REQUIRED_CONDITIONS = {
"aws:SourceArn",
"aws:SourceAccount",
"aws:SourceOrgID",
"aws:SourceOrgPaths",
}


def policy(resource):
bucket_policy = resource.get("Policy")
if bucket_policy is None:
return True # Pass if there is no bucket policy

policy_statements = json.loads(bucket_policy).get("Statement", [])
for statement in policy_statements:
# Check if the statement includes a service principal and allows access
principal = statement.get("Principal", {})
if "Service" in principal and statement["Effect"] == "Allow":
conditions = statement.get("Condition", {})
# Flatten nested condition keys (e.g., inside "StringEquals")
flat_condition_keys = set()
for condition in conditions.values():
if isinstance(condition, dict):
flat_condition_keys.update(condition.keys())
# Check if any required condition key is present
if not REQUIRED_CONDITIONS.intersection(flat_condition_keys):
return False
return True
39 changes: 39 additions & 0 deletions policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
AnalysisType: policy
Filename: aws_s3_bucket_policy_confused_deputy.py
PolicyID: "AWS.S3.Bucket.PolicyConfusedDeputyProtection"
DisplayName: "S3 Bucket Policy Confused Deputy Protection for Service Principals"
Enabled: true
ResourceTypes:
- AWS.S3.Bucket
Tags:
- AWS
- Security Control
- Best Practices
Severity: High
Description: >
Ensures that S3 bucket policies with service principals include conditions to prevent the confused deputy problem.
Runbook: >
Update the bucket policy to include conditions such as aws:SourceArn, aws:SourceAccount,
aws:SourceOrgID, or aws:SourceOrgPaths when a service principal is specified.
Reference: https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html
Tests:
- Name: Compliant Policy with Service Principal and Condition
ExpectedResult: true
Resource:
{
"Policy": '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Service":"cloudtrail.amazonaws.com"},"Action":"s3:PutObject","Resource":"arn:aws:s3:::my-example-bucket/*","Condition":{"StringEquals":{"aws:SourceAccount":"123456789012"}}}]}'
}

- Name: Non-Compliant Policy with Service Principal and No Condition
ExpectedResult: false
Resource:
{
"Policy": '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Service":"cloudtrail.amazonaws.com"},"Action":"s3:PutObject","Resource":"arn:aws:s3:::my-example-bucket/*"}]}'
}

- Name: Policy without Service Principal
ExpectedResult: true
Resource:
{
"Policy": '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"AWS":"arn:aws:iam::123456789012:root"},"Action":"s3:GetObject","Resource":"arn:aws:s3:::my-example-bucket/*"}]}'
}

0 comments on commit 6224265

Please sign in to comment.