Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add IPv4 CIDR and IPv6 CIDR validations #367

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

langscot
Copy link

Implements #342. Adds separate validations for both IPv4 and IPv6 CIDR notations.

@fabian-hiller fabian-hiller added the enhancement New feature or request label Jan 14, 2024
@fabian-hiller fabian-hiller self-assigned this Jan 14, 2024
@fabian-hiller
Copy link
Owner

fabian-hiller commented Jan 14, 2024

Thank you for your contribution. I will review this PR in 1 or 2 weeks.

Note to myself: Check this before merging.

*/
export const IPV6_CIDR_REGEX =
// eslint-disable-next-line redos-detector/no-unsafe-regex, regexp/no-super-linear-backtracking -- false positive
/^s*((([\dA-Fa-f]{1,4}:){7}([\dA-Fa-f]{1,4}|:))|(([\dA-Fa-f]{1,4}:){6}(:[\dA-Fa-f]{1,4}|((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([\dA-Fa-f]{1,4}:){5}(((:[\dA-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([\dA-Fa-f]{1,4}:){4}(((:[\dA-Fa-f]{1,4}){1,3})|((:[\dA-Fa-f]{1,4})?:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([\dA-Fa-f]{1,4}:){3}(((:[\dA-Fa-f]{1,4}){1,4})|((:[\dA-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([\dA-Fa-f]{1,4}:){2}(((:[\dA-Fa-f]{1,4}){1,5})|((:[\dA-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([\dA-Fa-f]{1,4}:)(((:[\dA-Fa-f]{1,4}){1,6})|((:[\dA-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(:(((:[\dA-Fa-f]{1,4}){1,7})|((:[A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:)))(%.+)*(\/(\d|[1-9]\d|1[01]\d|12[0-8]))$/u;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't fully understand why the s* is needed here. What does it do?

Additionally, are we sure that both redos and no-super-linear-backtracking are false positives?

I haven't had the time to try https://2bdenny.github.io/ReScue/ & https://github.com/doyensec/regexploit, but if one of them shows a vulnerability too, we have 3 online and at least one offline tool showing it's vulnerable, which tells us it's certainly vulnerable.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will look into the vulnerabilities further. I’m not sure I understood them correctly, thanks for pointing this out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kurtextrem thanks for the review, i agree with you. We should not use Regex which doesn't pass those regex exploits checking tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants