Skip to content

fix(query): fix fp for s3_bucket_access_to_any_principal#7518

Closed
cx-andre-pereira wants to merge 11 commits into
Checkmarx:masterfrom
cx-andre-pereira:False_positive_S3_Bucket_Access_to_Any_Principal
Closed

fix(query): fix fp for s3_bucket_access_to_any_principal#7518
cx-andre-pereira wants to merge 11 commits into
Checkmarx:masterfrom
cx-andre-pereira:False_positive_S3_Bucket_Access_to_Any_Principal

Conversation

@cx-andre-pereira

Copy link
Copy Markdown
Contributor

Reason for Proposed Changes

  • Currently the query s3_bucket_access_to_any_principal can only determine if a "Principal" has permissions "*" or "s3:*" from YAML files if it has no "!If" statements associated.
  • With "!If" statements the flag will trigger every time due to lack of specific case handling . The query does not take into consideration the existence of If´s.
  • The If structure is as follows : (YAML)
    - <CONDITION>
    - <THEN>
    - <ELSE>
  • The values for <THEN> and <ELSE> require scanning that is currently inexistent. Either of these values can have meaningful policy statements for this query.

Proposed Changes

  • Introduced two helper functions (handle_if_statements and handle_if_statement) to correctly traverse and extract valid policy statements from structures resulting from !If conditions.

  • This improves the query’s accuracy by avoiding false positives when Principal is not actually "*" in resolved policy logic, and also guarantees the verifications of each and every meaningful object in the if statement be it a <THEN> or <ELSE> associated object.

I submit this contribution under the Apache-2.0 license.

@cx-andre-pereira cx-andre-pereira requested a review from a team as a code owner July 4, 2025 17:01
@github-actions github-actions Bot added community Community contribution query New query feature labels Jul 4, 2025
@cx-andre-pereira cx-andre-pereira changed the title False positive s3 bucket access to any principal fix(query): fix fp for s3_bucket_access_to_any_principal Jul 7, 2025

@cx-artur-ribeiro cx-artur-ribeiro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi André!
First of all, nice work!
Can you take a look at my comments and see if you agree? The suggestions are just quality of life changes, the query seems to work as expected, great job!

Thanks for the work done!

Comment thread assets/queries/cloudFormation/aws/s3_bucket_access_to_any_principal/query.rego Outdated
Comment thread assets/queries/cloudFormation/aws/s3_bucket_access_to_any_principal/query.rego Outdated

@cx-artur-ribeiro cx-artur-ribeiro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@cx-andre-pereira cx-andre-pereira deleted the False_positive_S3_Bucket_Access_to_Any_Principal branch July 16, 2025 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community Community contribution query New query feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants