fix(query): fix FP results on "IAM policy allows for data exfiltration" CloudFormation and Terraform queries#8030
Open
cx-andre-pereira wants to merge 4 commits intoCheckmarx:masterfrom
Conversation
…iam_policy data exfiltration queries
…45579_FP_on_IAM_Policy_Allows_For_Data_Exfiltration
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #7960
Reason for Proposed Changes
The current implementation of the 2 queries :
Is not restrictive enough leading to many False Positive results.
Both in the Terraform query's
aws_iam_policyand the CloudFormation query's 5 target "AWS::IAM::..." type resources the "Resource" field is never checked to ensure exposition is global, other queries on both platforms already perform this exact check like "iam_policy_grants_full_permissions" for Terraform and the "iam_policies_with_full_privileges" for CloudFormation.Without this check there is no reason to flag a given sample, it must expose all resources for the target vulnerability to be valid. On the relevant Cloudsplaining documentation this is inferred by the
without resource constraintsexcerpt.When I originally implemented these queries in feat(query): implements "iam policy allows for data exfiltration" - terraform/aws & cloudformation/aws #7631 all samples contained the relevant "
Resource: "*"" field/value but the queries themselves would not check for this, the negative tests also did not include samples for the scenario of missing said field/value, it was simply unaccounted for.Proposed Changes
Implemented the missing check, identical to similar queries :
common_lib.equalsOrInArray(statement.Resource, "*"), with the exception of the Terraform's "data" resources where the target field is slightly different :common_lib.equalsOrInArray(statement.resources, "*")Added negative tests to show the relevant check is being done, the new negative tests avoid triggering the query by having a sample with a "Resource" value of
arn:aws:dynamodb:us-east-1:123456789012:table/Usersover the permissive wildcard*.Slight changes to positive tests were done to show that array search is supported for both queries:
I made the choice to test these only on positive tests since the only way to know the check is working for the array search is to have the query flag precisely because it managed to check that inside the array the wildcard target was included.
I submit this contribution under the Apache-2.0 license.