Opened 7 months ago

Closed 7 months ago

#24082 closed defect (fixed)

uninitialized value in networkstatus_parse_vote_from_string() via fuzz_consensus.c

Reported by: catalyst Owned by:
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Brian Carpenter (Geeknik Labs) reported this MemorySanitizer warning (clang-6.0.0-trunk) via HackerOne report_id 276253:

==27381==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x555dedc903dc in networkstatus_parse_vote_from_string /root/tor/src/or/routerparse.c:3533:7
    #1 0x555ded737b33 in fuzz_main /root/tor/src/test/fuzz/fuzz_consensus.c:66:8
    #2 0x555ded736fc0 in main /root/tor/src/test/fuzz/fuzzing_common.c:179:3
    #3 0x7f0d938633f0 in __libc_start_main /build/glibc-mXZSwJ/glibc-2.24/csu/../csu/libc-start.c:291
    #4 0x555ded6c4a19 in _start (/root/tor/src/test/fuzz/fuzz-consensus+0x71a19)

  Uninitialized value was created by a heap allocation
    #0 0x555ded6ed77d in __interceptor_malloc (/root/tor/src/test/fuzz/fuzz-consensus+0x9a77d)
    #1 0x555dedf94f27 in tor_malloc_ /root/tor/src/common/util.c:150:12
    #2 0x555ded7373aa in global_init /root/tor/src/test/fuzz/fuzzing_common.c:110:18
    #3 0x555ded73689a in main /root/tor/src/test/fuzz/fuzzing_common.c:140:3
    #4 0x7f0d938633f0 in __libc_start_main /build/glibc-mXZSwJ/glibc-2.24/csu/../csu/libc-start.c:291

SUMMARY: MemorySanitizer: use-of-uninitialized-value /root/tor/src/or/routerparse.c:3533:7 in networkstatus_parse_vote_from_string
Exiting

By code inspection this appears to be an actual uninitialized value that's specific to the fuzzing support framework.

global_init() allocates mock_options using tor_malloc() rather than tor_malloc_zero(), so it's entirely uninitialized, and nothing else I see initializes it afterward. networkstatus_parse_vote_from_string() then retrieves it using get_options()->TestingTorNetwork (and doesn't check any other fields).

I think we should fix this to reduce the noise produced by static analyzers and sanitizers.

Child Tickets

Change History (3)

comment:1 Changed 7 months ago by nickm

Status: newneeds_review

Surprising that this hadn't shown up before!

I've tried the obvious fix as bug24082_032.

comment:2 Changed 7 months ago by catalyst

Status: needs_reviewmerge_ready

Brian confirms the fix works.

comment:3 Changed 7 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

merged to 0.3.2 and forward!

Note: See TracTickets for help on using tickets.