#28312 closed defect (duplicate)

Tor exits when configuring a PidFile containing ~

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: 0.3.2.1-alpha
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by teor)

The patch in #28298 may avoids the "~" bug in DataDirectory, but adding ~ to many other configured path makes tor exit:

tor DisableNetwork 1 PidFile "~/.torpid"

with this error:

Nov 05 00:07:29.465 [notice] Tor 0.3.4.9 (git-4ac3ccf2863b86e7) running on Darwin with Libevent 2.1.8-stable, OpenSSL 1.0.2p, Zlib 1.2.11, Liblzma 5.2.4, and Libzstd 1.3.7.
Nov 05 00:07:29.466 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
Nov 05 00:07:29.467 [notice] Configuration file "/usr/local/etc/tor/torrc" not present, using reasonable defaults.
Nov 05 00:07:29.473 [warn] Path for PidFile (~/.torpid) is relative and will resolve to /Users/base/tor-034/build-c/~/.torpid. Is this what you wanted?
Nov 05 00:07:29.475 [notice] Scheduler type KISTLite has been enabled.
Nov 05 00:07:29.475 [notice] Opening Socks listener on 127.0.0.1:9050
Nov 05 00:07:29.000 [warn] Unable to open "~/.torpid" for writing: No such file or directory
Nov 05 00:07:29.000 [err] Unable to write PIDFile "~/.torpid"
Nov 05 00:07:29.000 [err] set_options: Bug: Acting on config options left us in a broken state. Dying. (on Tor 0.3.4.9 4ac3ccf2863b86e7)
Nov 05 00:07:29.000 [err] Reading config failed--see warnings above.

Child Tickets

Change History (6)

comment:1 Changed 12 months ago by arma

First issue is that our PidFile handling never calls expand_filename(). If we want it to, we could fix that. We probably should.

Also, this might be a good time to wonder if there is a better way to call expand_filename more uniformly on config options that should expect it. Maybe want a new config type FILENAME, rather than the current ambiguous choice STRING ? Oh wait, we *have* one of those already, e.g. DirPortFrontPage is one. So the bug here is that PidFile is a STRING not a FILENAME? And the second bug is that we don't uniformly run expand_filename() on config options of type FILENAME?

comment:2 in reply to:  description Changed 12 months ago by arma

Replying to teor:

> tor(79983,0x7fffa9c06380) malloc: *** error for object 0x7: pointer being freed was not allocated
> *** set a breakpoint in malloc_error_break to debug
> Abort trap: 6
> Exit 134

I can't reproduce this part. Are you sure you did a clean build? Is there something wrong with your osx? (Also, you're on 0.3.4.8, when there's an 0.3.4.9 out.)

comment:3 Changed 12 months ago by teor

It is reproducible, here's the AddressSanitizer output:

Nov 05 00:02:49.018 [notice] Tor 0.3.4.8 (git-da95b91355248ad8) running on Darwin with Libevent 2.1.8-stable, OpenSSL 1.0.2p, Zlib 1.2.11, Liblzma 5.2.4, and Libzstd 1.3.7.
...
=================================================================
==3252==ERROR: AddressSanitizer: heap-use-after-free on address 0x61d000003d48 at pc 0x00010cfc9888 bp 0x7ffee2d27350 sp 0x7ffee2d27348
READ of size 8 at 0x61d000003d48 thread T0
    #0 0x10cfc9887 in or_options_free_ config.c:968
    #1 0x10cfc9ac7 in config_free_all config.c:997
    #2 0x10d1f9195 in tor_free_all main.c:3677
    #3 0x10d1f9a54 in tor_run_main main.c:4258
    #4 0x10d3c5e20 in tor_main tor_api.c:84
    #5 0x10ced8efa in main tor_main.c:32
    #6 0x7fff710f6014 in start (libdyld.dylib:x86_64+0x1014)

0x61d000003d48 is located 200 bytes inside of 2256-byte region [0x61d000003c80,0x61d000004550)
freed by thread T0 here:
    #0 0x10e27410d in wrap_free (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x5710d)
    #1 0x10cfe7a16 in options_init_from_string config.c:5534
    #2 0x10cfe5b83 in options_init_from_torrc config.c:5280
    #3 0x10d1f88ff in tor_init main.c:3524
    #4 0x10d1f9a45 in tor_run_main main.c:4256
    #5 0x10d3c5e20 in tor_main tor_api.c:84
    #6 0x10ced8efa in main tor_main.c:32
    #7 0x7fff710f6014 in start (libdyld.dylib:x86_64+0x1014)

previously allocated by thread T0 here:
    #0 0x10e273f53 in wrap_malloc (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x56f53)
    #1 0x10d46b469 in tor_malloc_ util.c:150
    #2 0x10d46b520 in tor_malloc_zero_ util.c:178
    #3 0x10cfe6c8e in options_init_from_string config.c:5383
    #4 0x10cfe5b83 in options_init_from_torrc config.c:5280
    #5 0x10d1f88ff in tor_init main.c:3524
    #6 0x10d1f9a45 in tor_run_main main.c:4256
    #7 0x10d3c5e20 in tor_main tor_api.c:84
    #8 0x10ced8efa in main tor_main.c:32
    #9 0x7fff710f6014 in start (libdyld.dylib:x86_64+0x1014)

SUMMARY: AddressSanitizer: heap-use-after-free config.c:968 in or_options_free_
Shadow bytes around the buggy address:
  0x1c3a00000750: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c3a00000760: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c3a00000770: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c3a00000780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c3a00000790: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x1c3a000007a0: fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd fd
  0x1c3a000007b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c3a000007c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c3a000007d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c3a000007e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c3a000007f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==3252==ABORTING
Abort trap: 6
Exit 134

comment:4 Changed 12 months ago by teor

Resolution: duplicate
Status: newclosed

Ah, that's #27708.

comment:5 in reply to:  1 ; Changed 12 months ago by teor

Description: modified (diff)
Keywords: memory-safety regression security-low removed
Milestone: Tor: 0.3.5.x-finalTor: unspecified
Priority: HighMedium
Resolution: duplicate
Status: closedreopened
Summary: Crash when configuring a PidFile containing ~Tor exits when configuring a PidFile containing ~

Replying to arma:

First issue is that our PidFile handling never calls expand_filename(). If we want it to, we could fix that. We probably should.

Also, this might be a good time to wonder if there is a better way to call expand_filename more uniformly on config options that should expect it. Maybe want a new config type FILENAME, rather than the current ambiguous choice STRING ? Oh wait, we *have* one of those already, e.g. DirPortFrontPage is one. So the bug here is that PidFile is a STRING not a FILENAME? And the second bug is that we don't uniformly run expand_filename() on config options of type FILENAME?

Let's deal with these issues, then.

comment:6 in reply to:  5 Changed 12 months ago by arma

Parent ID: #28298
Resolution: duplicate
Status: reopenedclosed

Replying to teor:

Let's deal with these issues, then.

I'm going to re-close this ticket as a duplicate of #27708, and we can do the above "use expand_filename more consistently" work in #28311.

Note: See TracTickets for help on using tickets.