Opened 3 weeks ago

Last modified 30 hours ago

#31003 needs_review defect

heap-use-after-free src/feature/nodelist/routerlist.c:704 in router_get_by_descriptor_digest

Reported by: dgoulet Owned by: nickm
Priority: High Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-crash, tor-hs, 041-backport, 040-backport?, 035-backport?, 041-should?, 041-regression?, 041-must
Cc: Actual Points: .1
Parent ID: Points:
Reviewer: Sponsor:

Description

Doing some HS DoS testing and on ctrl+c of my tor client (unmodified), this showed up.

Tor version 0.4.2.0-alpha-dev (git-6afe1b00c9c73b1b).

(info.log attached to the ticket)

==16279==ERROR: AddressSanitizer: heap-use-after-free on address 0x60e000002428 at pc 0x559683ab9839 bp 0x7ffff3007db0 sp 0x7ffff3007da0
READ of size 8 at 0x60e000002428 thread T0
    #0 0x559683ab9838 in router_get_by_descriptor_digest src/feature/nodelist/routerlist.c:704
    #1 0x559683aa2a12 in count_usable_descriptors src/feature/nodelist/nodelist.c:2388
    #2 0x559683aa2f75 in compute_frac_paths_available src/feature/nodelist/nodelist.c:2448
    #3 0x559683aaf204 in update_router_have_minimum_dir_info src/feature/nodelist/nodelist.c:2701
    #4 0x559683aaf204 in router_have_minimum_dir_info src/feature/nodelist/nodelist.c:2301
    #5 0x559683a52714 in can_client_refetch_desc src/feature/hs/hs_client.c:1184
    #6 0x559683a52714 in hs_client_refetch_hsdesc src/feature/hs/hs_client.c:1350
    #7 0x559683a56bc2 in retry_all_socks_conn_waiting_for_desc src/feature/hs/hs_client.c:298
    #8 0x559683a56bc2 in hs_client_dir_info_changed src/feature/hs/hs_client.c:1936
    #9 0x559683abab62 in routerlist_free_ src/feature/nodelist/routerlist.c:944
    #10 0x559683abab62 in routerlist_free_all src/feature/nodelist/routerlist.c:1429
    #11 0x5596838ce3f4 in tor_free_all src/app/main/shutdown.c:116
    #12 0x5596838cc0c4 in tor_run_main src/app/main/main.c:1358
    #13 0x5596838c86b8 in tor_main src/feature/api/tor_api.c:164
    #14 0x5596838c1dbf in main src/app/main/tor_main.c:32
    #15 0x7f6565a75b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)
    #16 0x5596838c7db9 in _start (/home/dgoulet/Documents/git/tor/src/app/tor+0x1ccdb9)

0x60e000002428 is located 8 bytes inside of 160-byte region [0x60e000002420,0x60e0000024c0)
freed by thread T0 here:
    #0 0x7f656659f75f in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10d75f)
    #1 0x559683ab6fa4 in routerlist_free_ src/feature/nodelist/routerlist.c:968
    #2 0x559683abab62 in routerlist_free_ src/feature/nodelist/routerlist.c:944
    #3 0x559683abab62 in routerlist_free_all src/feature/nodelist/routerlist.c:1429
    #4 0x5596838ce3f4 in tor_free_all src/app/main/shutdown.c:116
    #5 0x5596838cc0c4 in tor_run_main src/app/main/main.c:1358
    #6 0x5596838c86b8 in tor_main src/feature/api/tor_api.c:164
    #7 0x5596838c1dbf in main src/app/main/tor_main.c:32
    #8 0x7f6565a75b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)

previously allocated by thread T0 here:
    #0 0x7f656659fb58 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10db58)
    #1 0x559683c7804e in tor_malloc_ src/lib/malloc/malloc.c:45
    #2 0x559683c780e3 in tor_malloc_zero_ src/lib/malloc/malloc.c:71
    #3 0x559683ab99f1 in router_get_routerlist src/feature/nodelist/routerlist.c:812
    #4 0x559683aa4a88 in nodelist_assert_ok src/feature/nodelist/nodelist.c:853
    #5 0x559683aace28 in nodelist_set_consensus src/feature/nodelist/nodelist.c:662
    #6 0x559683a9b54a in networkstatus_set_current_consensus src/feature/nodelist/networkstatus.c:2137
    #7 0x559683a9beb9 in reload_consensus_from_file src/feature/nodelist/networkstatus.c:1761
    #8 0x559683a9bf8c in router_reload_consensus_networkstatus src/feature/nodelist/networkstatus.c:278
    #9 0x5596838cb17f in run_tor_main_loop src/app/main/main.c:1180
    #10 0x5596838cc0b4 in tor_run_main src/app/main/main.c:1328
    #11 0x5596838c86b8 in tor_main src/feature/api/tor_api.c:164
    #12 0x5596838c1dbf in main src/app/main/tor_main.c:32
    #13 0x7f6565a75b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)

SUMMARY: AddressSanitizer: heap-use-after-free src/feature/nodelist/routerlist.c:704 in router_get_by_descriptor_digest
Shadow bytes around the buggy address:
  0x0c1c7fff8430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1c7fff8440: 00 00 00 02 fa fa fa fa fa fa fa fa 00 00 00 00
  0x0c1c7fff8450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 05
  0x0c1c7fff8460: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c1c7fff8470: 00 00 00 00 00 00 00 00 00 00 06 fa fa fa fa fa
=>0x0c1c7fff8480: fa fa fa fa fd[fd]fd fd fd fd fd fd fd fd fd fd
  0x0c1c7fff8490: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0c1c7fff84a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1c7fff84b0: fd fd fd fa fa fa fa fa fa fa fa fa fd fd fd fd
  0x0c1c7fff84c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1c7fff84d0: fa fa fa fa fa fa fa fa 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
  Shadow gap:              cc

Child Tickets

Attachments (1)

info.log (22.5 KB) - added by dgoulet 3 weeks ago.

Download all attachments as: .zip

Change History (8)

Changed 3 weeks ago by dgoulet

Attachment: info.log added

comment:1 Changed 3 weeks ago by dgoulet

Keywords: tor-hs added

On special thing that happened is the tor client received 2 NEWNYMs before getting ctrl+c.

comment:2 Changed 3 weeks ago by nickm

Keywords: 041-backport? 041-should? 041-regression? added
Priority: MediumHigh

It looks like tor_free_all freed the object, but that it was later used. On the bright side tor_free_all() should only get invoked on shutdown. But we shouldn't get asan errors even then.

I'm going to assume this also affects 041 until we hear otherwise

comment:3 Changed 11 days ago by nickm

Keywords: 041-must? added
Owner: set to nickm
Status: newassigned

comment:4 Changed 11 days ago by nickm

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

comment:5 Changed 32 hours ago by nickm

Keywords: 041-deferred-20190719 added

Add 041-deferred-20190719 tag to 041-should tickets I am deferring today.

comment:6 Changed 32 hours ago by nickm

Keywords: 041-must added; 041-must? 041-deferred-20190719 removed

comment:7 Changed 30 hours ago by nickm

Actual Points: .1
Keywords: 041-backport 040-backport? 035-backport? added; 041-backport? removed
Status: assignedneeds_review

Oh yuck, the problem here as I see it is that routerlist_free() [which frees the entire routerlist object, and is only called on shutdown] is calling router_dir_info_changed(), which in turn is using the object that was just freed. This happens in turn because the global "routerlist" variable is still set at this point.

I think it might make more sense to remove the call to router_dir_info_changed(), but that could have subtle side effects I don't really understand. Instead, I'm making us set the routerlist global to NULL before we call routerlist_free().

I can't figure out why this happens in 0.4.1 and not before, so I've done a patch on 0.3.5 just in case, but I think we shouldn't backport too aggressively unless we're sure this causes no trouble.

Branch is bug31003_035; PR at https://github.com/torproject/tor/pull/1181 . I recommend that we merge to 0.4.1 and consider for later backport.

Note: See TracTickets for help on using tickets.