aws-cloudformation / aws-cloudformation/cloudformation-guard
[BUG] CIDR Netmasks are strings, not netmasks
- Dominant language
- Rust
- Stars
- 1.4k
- Forks
- 196
- Avg merge
- 3d 6h
- Merged PRs (30d)
- 5
Description
### Describe the bug
In the example [security ingress/egress rules](https://github.com/aws-cloudformation/cloudformation-guard/blob/main/guard-examples/security-policies/ec2-secgroup-inbound-outbound-access.guard), the only CIDR range that will flag as "open to the world" is the exact string of `0.0.0.0/0`. For example, `1.0.0.0/0` is also open to the world, but if I process my template where that's the ingress CIDR, I see:
```PASS rules
ec2-secgroup-inbound-outbound-access.guard/prevent_inbound_access_to_any_ip PASS
```
### To Reproduce
I am pretty sure if you change the [ingress test case](https://github.com/aws-cloudformation/cloudformation-guard/blob/main/guard-examples/security-policies/ec2-secgroup-inbound-outbound-access-tests.yaml#L47) to `1.0.0.0/0` it will pass the test when it should fail. EC2/VPC are perfectly happy to accept that as a CIDR, because it is valid syntactically. They will create an ingress CIDR range that, when you do `describe-security-groups` will show as `0.0.0.0/0`.
The same is probably true for IPv6 CIDR ranges (I haven't tested) because it looks like string matching on `::0`.
### Expected behavior
If we **must** treat IPv4 CIDRs as strings, can we just match strings that _end with `/0`_? That would be be a wee bit more robust.
Ideally we should parse CIDRs as data structures, extract the netmask as an integer, and complain on small numbers like `8` or less.
Contributor guide
Research direction
Start with guard-examples/security-policies/ec2-secgroup-inbound-outbound-access.guard and reproduce the issue by changing the ingress case in guard-examples/security-policies/ec2-secgroup-inbound-outbound-access-tests.yaml to 1.0.0.0/0. Trace how CIDR values are compared, including the mentioned IPv6 case, and confirm done when equivalent /0 ranges are rejected by the test.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- aws, rust
- Domain
- cloud, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100