[tor-bugs] #20871 [Core Tor/Torsocks]: Regression in Torsocks 2.2.0 breaks wget, among others

[Tor Bug Tracker & Wiki]

#20871: Regression in Torsocks 2.2.0 breaks wget, among others
-------------------------------+-----------------------------------
 Reporter:  cypherpunks        |          Owner:  dgoulet
     Type:  defect             |         Status:  needs_information
 Priority:  High               |      Milestone:
Component:  Core Tor/Torsocks  |        Version:
 Severity:  Major              |     Resolution:
 Keywords:                     |  Actual Points:
Parent ID:                     |         Points:
 Reviewer:                     |        Sponsor:
-------------------------------+-----------------------------------

Comment (by cypherpunks):

 It's not an exit relay issue because the error is instant and occurs no
 matter what exit is in use. I did find out what line is causing the
 problem though, and know a fix. I'm very confused why it works for you,
 because it seems like the logic is fundamentally flawed.

 In 2.2.0 in `src/lib/gethostbyname.c`, there's an if statement which
 checks if `name` is a valid IPv4 address. If it is valid IPv4 address, it
 should return 1, otherwise a negative value. What's weirder still is in
 `src/common/utils.c`, the `check_addr()` function is written in such a way
 that it cannot possibly return 0, only 1 or -1 (which both evaluate true
 in C). Later on, `utils_is_address_ipv4()`, which is just a wrapper for
 that function, is called. The if statement it's called in will always
 return true (because it can only possibly evaluate to 1 or -1), never
 false.

 Since `utils_is_address_ipv4()` cannot return 0, the if statement it is in
 will always assume that Torsocks has been passed an IP address directly,
 so it will never attempt to resolve it, and instead will try to convert it
 to network byte order. This is why it works when it's passed an IP address
 directly, but not a hostname. The only reason I can think of that it might
 work for you is if, for some reason, the domain is being resolved early
 and the IP is passed to `tsocks_gethostbyname()` (which is where the stuck
 if statement is located) on your system but not mine, bypassing the buggy
 logic.

 A fix would involve having `check_addr()` return 0 instead of -1 when it's
 passed a host which is a valid IPv4/IPv6 instead of a string hostname.
 Only `tests/unit/test_utils.c` would have to be changed because it
 explicitly checks for -1, but everything else would work.

 First two code blocks are from src/common/utils.c. The third is from
 src/lib/gethostbyname.c.

 {{{
 static int check_addr(const char *ip, int af)
 {
         int ret = 0;
         char buf[128];

         assert(ip);

         ret = inet_pton(af, ip, buf);
         if (ret != 1) {
                 ret = -1;
         }

         return ret;
 }
 }}}

 {{{
 int utils_is_address_ipv4(const char *ip)
 {
         return check_addr(ip, AF_INET);
 }
 }}}

 {{{
 /* Man page specifies that it can either be an hostname or IPv4 address.
  * If it's an address, go with it else try to resolve it through Tor. */
 if (utils_is_address_ipv4(name)) {
         if (inet_pton(AF_INET, name, &ip) <= 0) {
                 goto error;
         }
         /* "ip" now contains the network byte order of the address. */
 } else {
         /* We have a hostname so resolve it through Tor. */
         ret = tsocks_tor_resolve(AF_INET, name, &ip);
         if (ret < 0) {
                 goto error;
         }
 }
 }}}

 tl;dr
 `utils_is_address_ipv4()` is broken, as is the if statement which uses it,
 because it cannot return 0.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/20871#comment:2>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online