Opened 9 months ago

Last modified 6 months ago

#22469 new defect

tor should probably reject "0x00" in port range specifications

Reported by: toralf Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: 0.3.1.2-alpha
Severity: Normal Keywords: tor-relay torrc configuration intro ipv6
Cc: catalyst Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

something like

ExitPolicy reject6 [2a00:1450:4001:0815:0000:0000:0000:200e]:0x00

should spew an error message.

Child Tickets

Change History (14)

comment:1 Changed 9 months ago by Sebastian

Is the ticket meant to say "should not ignore"? Or can you clarify what you mean?

comment:2 Changed 9 months ago by catalyst

Milestone: Tor: unspecified

Also, what is invalid about that IPv6 address? Is this a behavior change in 0.3.1, or did it exist in prior releases?

comment:3 Changed 9 months ago by toralf

The stem lib spews an error if such an entry is put into torrc - so I realized this issue.
It seems that stem is much more restrict in validating such input than Tor.
I dunno if this issue happens in previous versions of Tor.

comment:4 Changed 9 months ago by toralf

Summary: tor should ignore invalid ipv6 address:port definitionstor should better validate invalid ipv6 address:port definitions

comment:5 Changed 9 months ago by catalyst

I'm not seeing anything obvious in stem's IPv6 address validation code that would reject that address, but perhaps I'm missing something. I checked is_valid_ipv6_address in stem/stem/util/connection.py on master. What's the exact error, and from what function?

comment:6 Changed 9 months ago by atagar

Hi catalyst. The trouble is that exit policies should always be of the form 'address:port' but tor accepts other things in its torrc. Here's the stem commit where I discuss it...

https://gitweb.torproject.org/stem.git/commit/?id=806cbcc

In particular tor accepts things like the following in its torrc...

ExitPolicy reject6 [2a00:1450:4001:081e:0000:0000:0000:200e]

But it shouldn't because it's missing the port (a ":443" or ":*" suffix). In teor's example above '0x00' isn't a valid port either.

comment:7 Changed 9 months ago by catalyst

tor.1.txt documents the port number as optional for ExitPolicy, so if it's a tor bug, it's a documented one. It doesn't explicitly say whether the 0x prefix is allowed for specifying hexadecimal port numbers. A comment in exit_policy.py says that the exitpattern spec in dir-spec.txt is stricter than what torrc allows. Is the problem that tor is propagating some exitpatterns to descriptors or to the control protocol without normalizing them to the stricter dir-spec.txt grammar?

(Also toralf not teor is the reporter of this ticket.)

comment:8 Changed 9 months ago by atagar

Oops! You're completely right....

https://gitweb.torproject.org/tor.git/tree/doc/tor.1.txt#n1634
https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n1230

I'll change stem to accept omitted ports. As for the confusion between teor and toralf you're completely right. I thought this was a ticket teor and I were discussing earlier this week - my bad. Completely disregard - omitted ports is a completely different issue from this. :P

comment:9 Changed 9 months ago by toralf

So, despite that the 0x00 is still an error, right ?

comment:10 Changed 9 months ago by atagar

yup

comment:11 Changed 8 months ago by teor

Parent ID: #22802

This is a benign case of #22802, where we use tor_parse_*() in automatic base mode.

comment:12 Changed 8 months ago by nickm

Keywords: tor-relay torrc configuration intro ipv6 added

comment:13 Changed 7 months ago by catalyst

Cc: catalyst added
Summary: tor should better validate invalid ipv6 address:port definitionstor should probably reject "0x00" in port range specifications

By code inspection it looks like 0x00 as the port might get accepted by parse_port_range() because tor_parse_long() gets called with a non-null next to detect a hyphen delimiting the maximum of a port range, but nothing seems to produce an error if some different character follows the first port number of the "range". i.e., 0x00 gets parsed as 0 followed by x00 as trailing garbage that gets ignored rather than producing an error. I haven't come up with a test for this yet.

comment:14 Changed 6 months ago by nickm

Parent ID: #22802
Note: See TracTickets for help on using tickets.