[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #23082 [Core Tor/Tor]: tor_addr_parse is overly permissive
#23082: tor_addr_parse is overly permissive
---------------------------------+----------------------------------
Reporter: dcf | Owner: rl1987
Type: defect | Status: needs_review
Priority: Medium | Milestone: Tor: unspecified
Component: Core Tor/Tor | Version: Tor: unspecified
Severity: Normal | Resolution:
Keywords: 032-unreached, ipv6 | Actual Points:
Parent ID: | Points:
Reviewer: | Sponsor:
---------------------------------+----------------------------------
Comment (by dcf):
Replying to [comment:14 rl1987]:
> https://github.com/torproject/tor/pull/302
Great, rl1987! If you don't mind review from the peanut gallery:
{{{#!diff
- if (src[0] == '[' && src[1])
+ if (src[0] == '[' && src[1] && src[strlen(src)-1] == ']') {
}}}
I think the condition `src[1]` is now redundant. It was meant to ensure
that the string contains at least 2 characters, but the newly added
condition also does that.
I note that the condition `brackets_detected != 0` is equivalent to `tmp
!= NULL`. But actually, I don't think I would try to remove the
`brackets_detected` variable, because it expresses clearly what it means.
I think the `tor_inet_pton(AF_INET6, ...)` line could use a comment saying
that IPv6 addresses are allowed to be with or without brackets.
{{{#!diff
+ /* Reject if src has needless trailing ':'. */
+ if (len > 2 && src[len - 1] == ':' && src[len - 2] != ':') {
+ result = -1;
+ } else if (tor_inet_pton(AF_INET6, src, &in6_tmp) > 0) {
}}}
I wonder, does this check for a trailing colon belong in `tor_inet_pton`
instead? Or is there a reason for `tor_inet_pton` to accept such inputs?
Maybe for compatibility with `inet_pton`? It's surprising to me that this
check is necessary; I would have assumed (wrongly) that the underlying
address parser would reject it.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/23082#comment:15>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs