aws-cloudformation / aws-cloudformation/cloudformation-guard

[BUG] CIDR Netmasks are strings, not netmasks

Open
#182 2 comments 2 reactions 0 assignees View on GitHub
bug needs-research
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.