Opened 4 weeks ago

Last modified 3 weeks ago

#32178 needs_revision defect

Tor adds trailing space character to log events

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version:
Severity: Minor Keywords: easy intro
Cc: Actual Points:
Parent ID: Points:
Reviewer: teor Sponsor:

Description

First noticed in #32164, where Tor Browser's "view of Tor logs" has a bonus space at the end of every line.

I believe it's Tor adding the space. This super simple hack:

diff --git a/src/feature/control/control_events.c b/src/feature/control/control_events.c
index 82ea943999..5ddfffeee8 100644
--- a/src/feature/control/control_events.c
+++ b/src/feature/control/control_events.c
@@ -1328,6 +1328,7 @@ control_event_logmsg(int severity, log_domain_mask_t domain, const char *msg)
       default: s = "UnknownLogSeverity"; break;
     }
     ++disable_log_messages;
+    printf("Sending \"650 %s %s\"\n", s, b?b:msg);
     send_control_event(event,  "650 %s %s\r\n", s, b?b:msg);
     if (severity == LOG_ERR) {
       /* Force a flush, since we may be about to die horribly */

shows it:

Sending "650 INFO circuit_free_(): Circuit 0 (id: 4) has been freed. "

I believe it comes from this snippet in control_event_logmsg():

    if (strchr(msg, '\n')) {
      char *cp;
      b = tor_strdup(msg);
      for (cp = b; *cp; ++cp)
        if (*cp == '\r' || *cp == '\n')
          *cp = ' ';
    }

That is, we send in log lines that have \n in them, and the function helpfully turns the \n into a ' '.

Bug went into Tor 0.1.1.1-alpha in commit c2f6fe9b (way back in the days of the v0 control protocol).

Child Tickets

Change History (6)

comment:1 Changed 4 weeks ago by nickm

Keywords: easy intro added
Milestone: Tor: unspecified
Severity: NormalMinor

comment:3 Changed 4 weeks ago by nickm

Milestone: Tor: unspecifiedTor: 0.4.3.x-final
Status: newneeds_review

comment:4 Changed 3 weeks ago by teor

Status: needs_reviewneeds_revision

Thanks for this patch, I made a suggestion on the ticket to avoid a memory safety issue.

comment:5 Changed 3 weeks ago by teor

Reviewer: teor

comment:6 Changed 3 weeks ago by teor

I left more comments on the pull request - we're almost there!

Note: See TracTickets for help on using tickets.