From 3b42eb3818873779f6dab25fc3dcb365ef9054e2 Mon Sep 17 00:00:00 2001 From: Daniel Barranquero <74871504+danibarranqueroo@users.noreply.github.com> Date: Tue, 26 Aug 2025 13:30:49 +0200 Subject: [PATCH] fix(s3): resource metadata error in `s3_bucket_shadow_resource_vulnerability` (#8572) --- prowler/CHANGELOG.md | 1 + ...hadow_resource_vulnerability.metadata.json | 2 +- ...s3_bucket_shadow_resource_vulnerability.py | 18 +-- ...cket_shadow_resource_vulnerability_test.py | 110 +++++++++++------- 4 files changed, 73 insertions(+), 58 deletions(-) diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index be4507fe9c..34d7dea22b 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -22,6 +22,7 @@ All notable changes to the **Prowler SDK** are documented in this file. - Improve AWS Security Hub region check using multiple threads [(#8365)](https://github.com/prowler-cloud/prowler/pull/8365) ### Fixed +- Resource metadata error in `s3_bucket_shadow_resource_vulnerability` check [(#8572)](https://github.com/prowler-cloud/prowler/pull/8572) --- diff --git a/prowler/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability.metadata.json b/prowler/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability.metadata.json index 4c9717f59e..8d071bbd34 100644 --- a/prowler/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability.metadata.json +++ b/prowler/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability.metadata.json @@ -3,7 +3,7 @@ "CheckID": "s3_bucket_shadow_resource_vulnerability", "CheckTitle": "Check for S3 buckets vulnerable to Shadow Resource Hijacking (Bucket Monopoly)", "CheckType": [ - "" + "Effects/Data Exposure" ], "ServiceName": "s3", "SubServiceName": "", diff --git a/prowler/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability.py b/prowler/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability.py index f18855da30..eb509b1c85 100644 --- a/prowler/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability.py +++ b/prowler/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability.py @@ -70,22 +70,12 @@ class s3_bucket_shadow_resource_vulnerability(Check): ) # Check if this bucket exists in another account if s3_client._head_bucket(bucket_name): - # Create a virtual bucket object for reporting - virtual_bucket = type( - "obj", - (object,), - { - "name": bucket_name, - "region": region, - "arn": f"arn:{s3_client.audited_partition}:s3:::{bucket_name}", - "tags": [], - }, - )() - - report = Check_Report_AWS(self.metadata(), resource=virtual_bucket) + report = Check_Report_AWS(self.metadata(), resource={}) report.region = region report.resource_id = bucket_name - report.resource_arn = virtual_bucket.arn + report.resource_arn = ( + f"arn:{s3_client.audited_partition}:s3:::{bucket_name}" + ) report.resource_tags = [] report.status = "FAIL" report.status_extended = f"S3 bucket {bucket_name} for service {service} is a known shadow resource that exists and is owned by another account." diff --git a/tests/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability_test.py b/tests/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability_test.py index 5013ff1cdd..9b80d6db35 100644 --- a/tests/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability_test.py +++ b/tests/providers/aws/services/s3/s3_bucket_shadow_resource_vulnerability/s3_bucket_shadow_resource_vulnerability_test.py @@ -13,13 +13,14 @@ from tests.providers.aws.utils import ( class Test_s3_bucket_shadow_resource_vulnerability: @mock_aws def test_no_buckets(self): - s3_client = mock.MagicMock - s3_client.buckets = {} aws_provider = set_mocked_aws_provider([AWS_REGION_US_EAST_1]) aws_provider.identity.identity_arn = f"arn:aws:iam::{AWS_ACCOUNT_NUMBER}:root" - s3_client = mock.MagicMock + + s3_client = mock.MagicMock() + s3_client.buckets = {} s3_client.provider = aws_provider s3_client._head_bucket = mock.MagicMock(return_value=False) + with ( mock.patch( "prowler.providers.common.provider.Provider.get_global_provider", @@ -36,28 +37,31 @@ class Test_s3_bucket_shadow_resource_vulnerability: check = s3_bucket_shadow_resource_vulnerability() result = check.execute() + assert len(result) == 0 @mock_aws def test_bucket_owned_by_account(self): - s3_client = mock.MagicMock + aws_provider = set_mocked_aws_provider([AWS_REGION_US_EAST_1]) + aws_provider.identity.identity_arn = f"arn:aws:iam::{AWS_ACCOUNT_NUMBER}:root" + bucket_name = f"sagemaker-{AWS_REGION_US_EAST_1}-{AWS_ACCOUNT_NUMBER}" - s3_client.audited_account_id = AWS_ACCOUNT_NUMBER - s3_client.audited_identity_arn = f"arn:aws:iam::{AWS_ACCOUNT_NUMBER}:root" + + s3_client = mock.MagicMock() s3_client.audited_canonical_id = AWS_ACCOUNT_NUMBER + s3_client.audited_partition = "aws" s3_client.buckets = { bucket_name: Bucket( name=bucket_name, arn=f"arn:aws:s3:::{bucket_name}", region=AWS_REGION_US_EAST_1, owner_id=AWS_ACCOUNT_NUMBER, + tags=[{"Key": "Environment", "Value": "test"}], ) } - aws_provider = set_mocked_aws_provider([AWS_REGION_US_EAST_1]) - aws_provider.identity.identity_arn = f"arn:aws:iam::{AWS_ACCOUNT_NUMBER}:root" - s3_client = mock.MagicMock s3_client.provider = aws_provider s3_client._head_bucket = mock.MagicMock(return_value=False) + with ( mock.patch( "prowler.providers.common.provider.Provider.get_global_provider", @@ -74,32 +78,41 @@ class Test_s3_bucket_shadow_resource_vulnerability: check = s3_bucket_shadow_resource_vulnerability() result = check.execute() + assert len(result) == 1 - assert result[0].status == "PASS" - assert ( - "is correctly owned by the audited account" in result[0].status_extended - ) + report = result[0] + + # Test all report attributes + assert report.status == "PASS" + assert report.region == AWS_REGION_US_EAST_1 + assert report.resource_id == bucket_name + assert report.resource_arn == f"arn:aws:s3:::{bucket_name}" + assert report.resource_tags == [{"Key": "Environment", "Value": "test"}] + assert "is correctly owned by the audited account" in report.status_extended + assert "SageMaker" in report.status_extended @mock_aws def test_bucket_not_predictable(self): - s3_client = mock.MagicMock + aws_provider = set_mocked_aws_provider([AWS_REGION_US_EAST_1]) + aws_provider.identity.identity_arn = f"arn:aws:iam::{AWS_ACCOUNT_NUMBER}:root" + bucket_name = "my-non-predictable-bucket" - s3_client.audited_account_id = AWS_ACCOUNT_NUMBER - s3_client.audited_identity_arn = f"arn:aws:iam::{AWS_ACCOUNT_NUMBER}:root" + + s3_client = mock.MagicMock() s3_client.audited_canonical_id = AWS_ACCOUNT_NUMBER + s3_client.audited_partition = "aws" s3_client.buckets = { bucket_name: Bucket( name=bucket_name, arn=f"arn:aws:s3:::{bucket_name}", region=AWS_REGION_US_EAST_1, owner_id=AWS_ACCOUNT_NUMBER, + tags=[{"Key": "Project", "Value": "test-project"}], ) } - aws_provider = set_mocked_aws_provider([AWS_REGION_US_EAST_1]) - aws_provider.identity.identity_arn = f"arn:aws:iam::{AWS_ACCOUNT_NUMBER}:root" - s3_client = mock.MagicMock s3_client.provider = aws_provider s3_client._head_bucket = mock.MagicMock(return_value=False) + with ( mock.patch( "prowler.providers.common.provider.Provider.get_global_provider", @@ -116,18 +129,25 @@ class Test_s3_bucket_shadow_resource_vulnerability: check = s3_bucket_shadow_resource_vulnerability() result = check.execute() + assert len(result) == 1 - assert result[0].status == "PASS" - assert "is not a known shadow resource" in result[0].status_extended + report = result[0] + + # Test all report attributes + assert report.status == "PASS" + assert report.region == AWS_REGION_US_EAST_1 + assert report.resource_id == bucket_name + assert report.resource_arn == f"arn:aws:s3:::{bucket_name}" + assert report.resource_tags == [{"Key": "Project", "Value": "test-project"}] + assert "is not a known shadow resource" in report.status_extended @mock_aws def test_shadow_resource_in_other_account(self): # Mock S3 client with no buckets in current account s3_client = mock.MagicMock() s3_client.buckets = {} - s3_client.audited_account_id = AWS_ACCOUNT_NUMBER - s3_client.audited_identity_arn = f"arn:aws:iam::{AWS_ACCOUNT_NUMBER}:root" s3_client.audited_canonical_id = AWS_ACCOUNT_NUMBER + s3_client.audited_identity_arn = f"arn:aws:iam::{AWS_ACCOUNT_NUMBER}:root" s3_client.audited_partition = "aws" # Mock regional clients - this is what the check uses to determine regions to test @@ -183,27 +203,31 @@ class Test_s3_bucket_shadow_resource_vulnerability: finding.status == "FAIL" and "shadow resource" in finding.status_extended ): - if ( - "aws-glue-assets" in finding.status_extended - and "Glue" in finding.status_extended - ): - found_services.add("Glue") - assert "us-west-2" in finding.status_extended - elif ( - "sagemaker" in finding.status_extended - and "SageMaker" in finding.status_extended - ): - found_services.add("SageMaker") - assert "us-east-1" in finding.status_extended - elif ( - "aws-emr-studio" in finding.status_extended - and "EMR" in finding.status_extended - ): - found_services.add("EMR") - assert "eu-west-1" in finding.status_extended + # Test all report attributes for cross-account findings + assert finding.status == "FAIL" + assert finding.resource_id in shadow_resources + assert finding.resource_arn == f"arn:aws:s3:::{finding.resource_id}" + assert finding.resource_tags == [] - # Verify common attributes - assert "owned by another account" in finding.status_extended + # Determine service from resource_id and test exact status_extended + if "aws-glue-assets" in finding.resource_id: + service = "Glue" + expected_status = f"S3 bucket {finding.resource_id} for service {service} is a known shadow resource that exists and is owned by another account." + assert finding.status_extended == expected_status + found_services.add("Glue") + assert finding.region == "us-west-2" + elif "sagemaker" in finding.resource_id: + service = "SageMaker" + expected_status = f"S3 bucket {finding.resource_id} for service {service} is a known shadow resource that exists and is owned by another account." + assert finding.status_extended == expected_status + found_services.add("SageMaker") + assert finding.region == "us-east-1" + elif "aws-emr-studio" in finding.resource_id: + service = "EMR" + expected_status = f"S3 bucket {finding.resource_id} for service {service} is a known shadow resource that exists and is owned by another account." + assert finding.status_extended == expected_status + found_services.add("EMR") + assert finding.region == "eu-west-1" # Verify we found all expected services expected_services = {"Glue", "SageMaker", "EMR"}