carnal0wnage / weirdAAL

WeirdAAL (AWS Attack Library)
772 stars 92 forks source link

because sorting is better than not #53

Closed nvisium-jonn-callahan closed 6 years ago

carnal0wnage commented 6 years ago

@nvisium-jonn-callahan search_recon_by_key does sort when it returns results. see: https://github.com/carnal0wnage/weirdAAL/blob/master/libs/sql.py line115

does this do anything different?

nvisium-jonn-callahan commented 6 years ago

Yep, sorts the output alphabetically. That query is sorting by date checked via a ORDER BY datetime(checked_at). Because of this, I was getting output such as this:

ec2:DescribeVpcEndpointServices
ec2:DescribeVpcEndpoints
ec2:DescribeVpcPeeringConnections
ec2:DescribeVpcs
ec2:CreateVpc
ec2:DescribeVpnConnections
ec2:DescribeVpnGateways
ecr:DescribeRepositories
ecs:ListClusters
ecs:DescribeClusters
ecs:ListContainerInstances
ecs:ListTaskDefinitions
efs:DescribeFileSystems

This threw me for a loop as I was checking for ec2:create perms and scrolled past the large block of ec2:describe and missed it. You could modify the order by to sort by service then sub_service, but I wasn't sure where else that query was leveraged, so figured it'd be easiest to just sort the result set, especially considering you're not going to have a more than a few hundred items. This PR will instead dump output like this:

ec2.CreateImage
ec2.CreateKeyPair
ec2.CreateVolume
ec2.CreateVpc
ec2.DescribeAccountAttributes
ec2.DescribeAddresses
ec2.DescribeAvailabilityZones
carnal0wnage commented 6 years ago

if you run recon_all twice -putting the same stuff in the db twice will your code still give you the desired output sorted by time?

nvisium-jonn-callahan commented 6 years ago

I'm not following -- this PR sorts the output alphabetically regardless of the row order in either the DB or the result set. If sorting by time intentional, that's fine, but seems fairly counterintuitive. If anything, I think it'd be more sensible to sort the output of module_list_services_by_key() alphabetically and module_list_services_by_key_with_date() chronologically imo.

carnal0wnage commented 6 years ago

cool. i'm gonna merge this, then change how the sort with date is done because i want it to post the most recent first

carnal0wnage commented 6 years ago

actually nevermind i can live with how it is. thanks for the diff!