Skip to content

fix(query): fix fp for s3_bucket_access_to_any_principal #7518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 11 commits into from
Closed

fix(query): fix fp for s3_bucket_access_to_any_principal #7518

wants to merge 11 commits into from

Conversation

cx-andre-pereira
Copy link
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 requested changes Jul 9, 2025
View reviewed changes
Copy link
Contributor

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

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!

cx-artur-ribeiro
cx-artur-ribeiro previously approved these changes Jul 10, 2025
View reviewed changes
Copy link
Contributor

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

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
@cx-andre-pereira cx-andre-pereira mentioned this pull request Jul 16, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cx-artur-ribeiro cx-artur-ribeiro cx-artur-ribeiro approved these changes

Assignees

@cx-andre-pereira cx-andre-pereira

Labels
community Community contribution query New query feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cx-andre-pereira @cx-artur-ribeiro