Opened 6 months ago

Closed 4 months ago

#31466 closed defect (fixed)

Consider demoting ".exit is disabled" log message to info

Reported by: asn Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: consider-backport-after-0423, bug-bounty, hackerone, tor-security?, 042-should 029-backport? 035-backport 040-backport 041-backport BugSmashFund dgoulet-merge
Cc: Actual Points: .1
Parent ID: Points:
Reviewer: asn Sponsor:

Description

  /* Check for whether this is a .exit address.  By default, those are
   * disallowed when they're coming straight from the client, but you're
   * allowed to have them in MapAddress commands and so forth. */
  if (!strcmpend(socks->address, ".exit")) {
    log_warn(LD_APP, "The  \".exit\" notation is disabled in Tor due to "
             "security risks.");
    control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s",
                                escaped(socks->address));
    out->end_reason = END_STREAM_REASON_TORPROTOCOL;
    out->should_close = 1;
    return;
  }

The above log message can be remotely triggered by websites and some people consider this a security issue: https://github.com/brave/brave-browser/issues/4629

This has also been reported to our h1 bug bounty program.

We should consider fixing this one particular instance, but I bet there is more of this lying around.

Child Tickets

Change History (11)

comment:1 Changed 6 months ago by nickm

Keywords: 042-should added

comment:2 Changed 5 months ago by nickm

Owner: set to nickm
Status: newaccepted

I think it's more appropriate to use a rate-limiter.

comment:3 Changed 5 months ago by nickm

Actual Points: .1
Keywords: 029-backport? 035-backport 040-backport 041-backport BugSmashFund added

See branch bug31466_029 with PR at https://github.com/torproject/tor/pull/1342 .

There's a merge conflict with 0.3.5, because of the removal of AllowDotExit. See bug31466_035 for a resolution to that conflict, with a PR at https://github.com/torproject/tor/pull/1343 .

This latter branch merges cleanly to master. To see CI running on master, the branch id is bug31466_042 with a PR at https://github.com/torproject/tor/pull/1344 .

I decided to go for a rate-limit here instead of demoting the log message's severity. We do want people to see this message if they are going to attempt to use the obsolete .exit notation... we just don't want to spam their logs with it.

I'll put this in needs_review once CI is happy.

comment:4 Changed 5 months ago by nickm

Status: acceptedneeds_review

comment:5 Changed 5 months ago by asn

Reviewer: asn

comment:6 Changed 5 months ago by asn

Status: needs_reviewmerge_ready

Looks good to me!

comment:7 Changed 5 months ago by nickm

Keywords: dgoulet-merge added

comment:8 Changed 5 months ago by dgoulet

Milestone: Tor: 0.4.2.x-finalTor: 0.4.1.x-final

PR 1344 merged. Moving to 041 milestone for backport.

comment:9 Changed 4 months ago by nickm

Milestone: Tor: 0.4.1.x-finalTor: 0.4.0.x-final

Backported to 0.4.1.

comment:10 Changed 4 months ago by teor

Keywords: consider-backport-after-0423 added

Let's wait until the next alpha to backport further

comment:11 Changed 4 months ago by teor

Milestone: Tor: 0.4.0.x-finalTor: 0.2.9.x-final
Resolution: fixed
Status: merge_readyclosed

Merged to 0.2.9 and later.
Merged #31107, #31466, #30916, #31408, #31837, and #31897 together.

Note: See TracTickets for help on using tickets.