Skip to content
New issue

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

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

Already on GitHub? # to your account

policyuniverse & Bucket Sniping #142

Open
tweedge opened this issue Aug 8, 2021 · 4 comments
Open

policyuniverse & Bucket Sniping #142

tweedge opened this issue Aug 8, 2021 · 4 comments

Comments

@tweedge
Copy link
Contributor

tweedge commented Aug 8, 2021

Howdy! I noticed that policyuniverse doesn't detect possible bucket sniping - i.e. when a bucket it not owned by the account you expect it to be, as S3 ARNs don't contain account information. statement21 in test_statement.py is a great example:

statement21 = dict(
    Effect="Allow",
    Principal="*",
    Action=["rds:*"],
    Resource="*",
    Condition={"StringEquals": {"AWS:SourceArn": "arn:aws:s3:::mybucket"}},
)

This would potentially allow an external party to access data if the mybucket bucket was deleted and then claimed by an external party, or if the account owner was using predictable bucket names (chris-bucket-us-west-1) and an external party claimed that bucket before they did, etc.

This would differ from policyuniverse's current understanding of what it means for something to be publicly exposed - extending from "this is definitely publicly exposed" to "this could be publicly exposed." I certainly understand if this isn't a change you'd be interested in adding because of that!

However if it is something you'd consider adding, I've tinkered around and found two solutions which could be implemented - one hides booleans here and there throughout statement.py, and adds a little evaluation logic to is_internet_accessible(). This is less readable (basically: flagging any use of unique conditions, principals, etc. since we're iterating through them anyway), but works well, and doesn't waste cycles. Another option would be to put this in its own function, duplicating some of the is_internet_accessible implementation at the cost of some waste (as this would cause a second loop through all conditions statements, principals, etc.), but is much more readable.

Let me know, happy to put a CR up for this, or happy without it 😁

@tweedge
Copy link
Contributor Author

tweedge commented Aug 8, 2021

Ruminating, could also split this out into a possibly_internet_accessible() function? Food for thought :P

@scriptsrc
Copy link
Contributor

scriptsrc commented Aug 8, 2021 via email

@scriptsrc
Copy link
Contributor

scriptsrc commented Aug 8, 2021 via email

@tweedge
Copy link
Contributor Author

tweedge commented Sep 16, 2021

A WIP CR is up but I'm not super satisfied with it - if you can give it a review and provide thoughts that'd be great!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants