Opened 5 years ago

Closed 5 years ago

#13071 closed defect (fixed)

[patch] tor 0.2.6 sometimes fails to escape logged directory requests

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version: Tor: 0.2.5.5-alpha
Severity: Keywords: 025-backport tor-relay
Cc: nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

tor 0.2.6 (git Jul - Sep 2014) sometimes writes (parts of) directory requests directly to the log without escaping them. This can lead to arbitrary characters being written to the log.

The attached test case should be run using:
cat tor_log_bell.http | nc -c tor-directory-ip tor-directory-port
A url-encoded version of the file is supplied in case the random garbage turns into sensible garbage in transit.

Running this test case on a tor directory logging "[debug] {DIRSERV}" would normally write a URL that ends in random garbage (including the BEL character) to the log. The bell makes the failure of the test easy to identify when reading the log from a terminal with an audible or visual bell.

After applying this patch, tor writes:

(time and date) [debug] {DIRSERV} int directory_handle_command_get(dir_connection_t *, const char *, const char *, size_t)(): rewritten url as '"/tor/server/fp/\014\303\266\302\220\302\2379L(\303\274\302\266\"\303\200\303\220\302\210&\303\226\302\261\034\007\006\302\217\303\264\302\234\302\257\302\267\302\245\303\273\302\257\302\265o\030\302\210d7A\302\233<\302\263\302\2148H"'.

(No bell sound occurs.)

Child Tickets

Attachments (3)

escape-directory-log.patch (2.2 KB) - added by teor 5 years ago.
[patch] src/or/directory.c - escape() all directory request-derived strings
tor_log_bell.http (103 bytes) - added by teor 5 years ago.
test case - log random characters to debug DIRSERV log
tor_log_bell.http.encoded (204 bytes) - added by teor 5 years ago.
url encoded test case - log random characters to debug DIRSERV log

Download all attachments as: .zip

Change History (6)

Changed 5 years ago by teor

Attachment: escape-directory-log.patch added

[patch] src/or/directory.c - escape() all directory request-derived strings

Changed 5 years ago by teor

Attachment: tor_log_bell.http added

test case - log random characters to debug DIRSERV log

Changed 5 years ago by teor

Attachment: tor_log_bell.http.encoded added

url encoded test case - log random characters to debug DIRSERV log

comment:1 Changed 5 years ago by teor

Clarification: the patch only applies escape() to network-supplied directory requests logged in src/or/directory.c
Network-supplied data could be logged un-escaped in other files as well.

comment:2 Changed 5 years ago by nickm

Keywords: 025-backport tor-relay added
Milestone: Tor: 0.2.6.x-final
Status: newneeds_review

comment:3 Changed 5 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged to 0.2.5 and master. Thanks!

Note: See TracTickets for help on using tickets.