Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#19947 closed defect (fixed)

NULL %s in fmt string (dir_server_new() - routerlist.c)

Reported by: rubiate Owned by:
Priority: Medium Milestone: Tor: 0.2.8.x-final
Component: Core Tor/Tor Version: Tor: 0.2.8.1-alpha
Severity: Normal Keywords: regression?, 028-backport TorCoreTeam201608
Cc: cb@… Actual Points:
Parent ID: Points: 0.5
Reviewer: Sponsor:

Description

in src/or/routerlist.c:4545-4550 hostname passed to tor_asprintf() can be NULL such as when dir_server_new() is called from fallback_dir_server_new()

Found this because recent OpenBSD -current whinges about it in syslog:

tor: vfprintf %s NULL in "directory server at %s:%d"
last message repeated 88 times

Child Tickets

Attachments (1)

nullfmt.patch (662 bytes) - added by rubiate 3 years ago.
use hostname_ instead of hostname

Download all attachments as: .zip

Change History (7)

Changed 3 years ago by rubiate

Attachment: nullfmt.patch added

use hostname_ instead of hostname

comment:1 Changed 3 years ago by rubiate

Passing hostname_ instead would fix this as it always gets set to a value (either the same as hostname if it's not NULL, or the address).

comment:2 Changed 3 years ago by teor

Keywords: regression? 028-backport added
Milestone: Tor: 0.2.8.x-final
Points: 0.5
Status: newmerge_ready
Version: Tor: 0.2.8.1-alpha

Thanks for reporting this. I'm surprised we didn't catch it earlier in our unit tests.

It looks like we introduced this bug back in tor-0.2.4.7-alpha, but it only affected people who uses the FallbackDir option. When we added hard-coded fallback directories in 0.2.8.1-alpha, that triggered this bug on the default config.

I think this patch fixed the issue.
We'll need a short changes file.

I wonder if we should put it in 0.2.8.7, because passing NULL like this is a bug. It's definitely undefined behaviour, so it has potential security implications. Printing the contents of NULL to your log is bad, and could corrupt memory or cause crashes.

comment:3 Changed 3 years ago by teor

Here's my proposed changes file entry:

  o Minor bugfixes (fallback directories):
    - Avoid logging a NULL string pointer when loading fallback directory information.
      Fixes bug 19947; bugfix on 0.2.4.7-alpha and 0.2.8.1-alpha.
      Report and patch by "rubiate".

comment:4 Changed 3 years ago by nickm

Resolution: fixed
Status: merge_readyclosed

committed in 0ba05313d56e6737c73d003c84eeebdfd0d40398 !

actual-review-points: 0

comment:5 Changed 3 years ago by nickm

(Also, thanks!)

comment:6 Changed 3 years ago by nickm

Keywords: TorCoreTeam201608 added
Note: See TracTickets for help on using tickets.