fix(query): exclude private CIDRs from AWS small-public-network query#8065
Open
arpitjain099 wants to merge 1 commit into
Open
fix(query): exclude private CIDRs from AWS small-public-network query#8065arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
The Terraform query aws/sensitive_port_is_exposed_to_small_public_network flagged any ingress CIDR whose suffix was /25-/29 (or /121-/125 for IPv6) without checking whether the range is actually public. As a result private RFC1918 ranges such as 10.0.0.0/25, 192.168.0.0/26 and 172.16.0.0/27 (and IPv6 ULA fd00::/121) were reported as small public networks, a false positive. isSmallPublicNetwork now also requires the CIDR not to be private, reusing the existing common_lib.isPrivateIP helper (the same helper the wide_private_network twin query relies on). Genuinely public small ranges still flag. The positive fixtures were updated to use public small CIDRs (they previously used private ranges, which is exactly the false positive) and negative cases were added with private /25-/27 and ULA /121 ranges on a sensitive port to lock in the fix. Signed-off-by: arpitjain099 <arpitjain099@gmail.com>
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
1 participant
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 #8032
Reason for Proposed Changes
aws/sensitive_port_is_exposed_to_small_public_networkonly looked at the CIDR suffix (/25-/29for IPv4,/121-/125for IPv6) to decide a range was a "small public network". It never checked whether the range was actually public.10.0.0.0/25,192.168.0.0/26and172.16.0.0/27, as well as IPv6 ULA ranges likefd00::/121, were reported as small public networks. That is a false positive: exposing a sensitive port to a small private subnet is not the risk this query is meant to catch, and bug(terraform): sensitive port Is exposed to small public network is too broad #8032 reports the rule as too broad for exactly this reason.Proposed Changes
isSmallPublicNetworknow requires the CIDR to be small AND not private. The private check reuses the existingcommon_lib.isPrivateIPhelper (RFC191810.0.0.0/8,172.16.0.0/12,192.168.0.0/16and IPv6 ULAfc00::/8,fd00::/8), which is the same helper the siblingsensitive_port_is_exposed_to_wide_private_networkquery already uses, so the public/private boundary stays consistent across the two queries.203.0.113.0/25,198.51.100.0/26,8.8.8.0/27, public IPv6 GUA, etc.); they previously used private RFC1918/ULA ranges, which is the false positive itself, so they no longer belong in the positive set./25-/27and ULA/121ranges on a sensitive port and TCP, across all four resource shapes (aws_security_group,aws_vpc_security_group_ingress_rule,aws_security_group_rule, and theterraform-aws-modules/security-group/awsmodule), so the false positive is locked in.Tests run locally with
go test ./test/ -run 'TestQueries/terraform/aws/sensitive_port_is_exposed_to_small_public_network', plusTestQueriesContentandTestQueriesMetadata, all green. I also confirmed the new negative cases fail against the pre-fix query, so they are a real regression guard rather than vacuous.Note: #8032 mentions the Azure twin
azure/sensitive_port_is_exposed_to_small_public_networkhas the same issue. I scoped this PR to AWS only to keep the review focused; I am happy to follow up with the matching Azure fix in a separate PR.I submit this contribution under the Apache-2.0 license.