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

make vhost "looks like a CIDR" check more precise #920

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jesopo
Copy link
Member

@jesopo jesopo commented Feb 21, 2024

i don't know if this code was intentionally very imprecise but it's blocking a cloak we'd like to use that is quite obviously fine

@jesopo jesopo force-pushed the jess/less-bad-cidr-check branch 3 times, most recently from 61333cd to 955cbd8 Compare February 21, 2024 10:48
p++;
// if we found a /, and are now at the end of the vhost, that
// means we found no non-digits, so it looks too much like a CIDR
if (p2 != NULL && p[1] == '\0')
Copy link
Contributor

@dwfreed dwfreed Feb 23, 2024

Choose a reason for hiding this comment

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

This doesn't need storing a second copy of the strrchr return value. If the return value was NULL then p will still be NULL here; if the return value was not NULL then p will never be NULL either.

@jesopo
Copy link
Member Author

jesopo commented Feb 25, 2024

this PR may be a no-go anyway as it'd apparently require us to audit all the IRCds atheme can link with

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

Successfully merging this pull request may close these issues.

None yet

2 participants