Opened 4 years ago

Last modified 2 years ago

#16366 new defect

tor hangs for 30 seconds when parsing torrc ending in backslash-newline

Reported by: teor Owned by:
Priority: Low Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: 0.2.6.8
Severity: Normal Keywords: lorax tor-relay tor-client configuration newline osx torrc
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

When parsing torrc files that end in backslash then newline, tor hangs for approximately 30 seconds.

Examples:

ORPort \

HTTPProxy \

I have confirmed this issue in tor 0.2.6.8 and tor 0.2.7.1-alpha.

Note:

  • the eventual result is still a failed parse, which is correct
  • the issue is of delay/annoyance value at best
  • triggering the issue requires access to torrc

Child Tickets

TicketStatusOwnerSummaryComponent
#16372closedtor uses getaddrinfo even if DisableNetwork is setCore Tor/Tor

Attachments (1)

getaddrinfo.c (367 bytes) - added by teor 4 years ago.
Sample program that does getaddrinfo(\)

Download all attachments as: .zip

Change History (14)

comment:1 Changed 4 years ago by teor

This issue was generated using afl-fuzz 1.08b, and discovered using afl-cmin 1.82b.

comment:2 Changed 4 years ago by teor

This appears to be an address resolution issue.
Tor is hanging on this line of code in address.c:

    err = sandbox_getaddrinfo(name, NULL, &hints, &res);

For some reason, almost every other name or symbol either gets resolved quickly, or fails.
But \ takes 30 seconds to resolve. The resolution is attempted even if --DisableNetwork 1 is specified.

I'm using OS X, in case this behaviour is restricted to a particular platform.

The simplest command-line which can reproduce this issue for me is:

tor --verify-config --ORPort \\ -f /dev/null

comment:3 Changed 4 years ago by teor

Split the getaddrinfo call when DisableNetwork is set into #16372

Changed 4 years ago by teor

Attachment: getaddrinfo.c added

Sample program that does getaddrinfo(\)

comment:4 Changed 4 years ago by teor

The attached program performs a similar getaddrinfo call to the call tor makes when resolving a *Port config line.
On OS X, it hangs for 30 seconds.
On Linux, it returns immediately.

Also, on OS X, it only hangs once the SOCK_STREAM and AF_UNSPEC hints are set. If the hints are uninitialised (oops!), it returns immediately.

What does OS X think is at '\' ?

comment:5 Changed 4 years ago by nickm

I investigated a little, using dtruss (on osx) and strace (on Linux) and reading the source code (ugh, hairy). Here's what I found:

When you do getaddrinfo("
") on osx, it sends a query to the network, and never gets a reply.

But when you do getaddrinfo("
") on glibc, it never sends a query to the network at all; instead, it recognizes early that the name can't be valid.

So maybe this is not a Tor issue?

comment:6 Changed 4 years ago by teor

Oh, I completely agree, the attached test program causes the issue to occur on OS X, but not on Linux. And all it contains is a simple getaddrinfo call that any program could use.

I don't know if it's worth filtering obvious invalid addresses like \ and \n and \r or not. Since we are already parsing and validating address/port combinations, should we do a very simple validity check with the name part? (Do we already do some checks? Or do we leave everything before the colon to getaddrinfo?)

comment:7 Changed 4 years ago by teor

A correction to my previous post:
nickm wrote getaddrinfo("\\"), but it's rendered by trac as getaddrinfo("
").

Please ignore my references to newlines.

comment:8 Changed 4 years ago by teor

Can we run each string through string_is_valid_hostname or similar?

comment:9 in reply to:  8 Changed 4 years ago by yawning

Replying to teor:

Can we run each string through string_is_valid_hostname or similar?

Need a separate routine, if only to handle bracket delimited IPv6 addresses. string_is_valid_hostname is only really intended for validating FQDNs, since raw IPs are (for correctly implemented SOCKS clients) never passed as DOMAINNAME in the SOCKS protocol (which is the main consumer of the routine).

comment:10 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:11 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:12 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:13 Changed 2 years ago by nickm

Keywords: tor-relay tor-client configuration newline osx torrc added
Severity: Normal
Note: See TracTickets for help on using tickets.