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

#26488: Validate that DirAuthority address is IPv4 #181

Closed
wants to merge 2 commits into from

Conversation

Labels
None yet
Projects
None yet
3 participants
@rl1987
Copy link
Contributor

@rl1987 rl1987 commented Jun 26, 2018

https://trac.torproject.org/projects/tor/ticket/26488

}

address = tor_strndup(addrport, addrport_sep - addrport);
if (!string_is_valid_ipv4_address(address)) {
Copy link
Contributor

@dgoulet-tor dgoulet-tor Jul 2, 2018

I think we could avoid the dup() entirely here and simply passing addrport_sep - addrport to this function. This avoid us an allocation and free.

Copy link
Contributor Author

@rl1987 rl1987 Jul 2, 2018

That doesn't work, as it tries feeding the address to tor_inet_pton and sees if parsing succeeded. It doesn't like the separating colon.

Copy link
Contributor

@dgoulet-tor dgoulet-tor Jul 3, 2018

Isn't addrportsep - addrport is only the address without the semicolon?

Copy link
Contributor Author

@rl1987 rl1987 Jul 3, 2018

Umm no? It's number of bytes from the start of addrport to colon (pointer value in const char *addrport_sep) - a length of address alone (without colon and port). string_is_valid_ipv4_address() only takes the string and does not have a second parameter that says "check this much of the input". That's why I'm using tor_strndup() to copy only the part I can check with string_is_valid_ipv4_address().

Maybe there's some code clarity improvement to be made?

Copy link
Contributor

@dgoulet-tor dgoulet-tor Jul 3, 2018

Ahhhh! yes ok I get it. That is fine then. Lets merge this!

@coveralls
Copy link

@coveralls coveralls commented Jul 2, 2018

Coverage Status

Coverage decreased (-0.02%) to 59.875% when pulling f628d58 on rl1987:bug26488 into b556894 on torproject:master.

@rl1987 rl1987 closed this Jul 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment