Opened 3 years ago

Closed 3 years ago

#22924 closed defect (fixed)

signed integer overflow in unit tests crashes hardened build on 32-bit trusty

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


If I build on gcc with --enable-fragile-hardening on ubuntu trusty, I get a crash in dir/download_status_random_backoff :

#0  0xf7fd9d70 in __kernel_vsyscall ()
#1  0xf6599827 in raise () from /lib/i386-linux-gnu/
#2  0xf659cc53 in abort () from /lib/i386-linux-gnu/
#3  0x56e5e16c in __addvsi3 ()
#4  0x5679e366 in download_status_random_backoff_helper (
    min_delay=min_delay@entry=0, max_delay=max_delay@entry=2147483647)
    at src/test/test_dir.c:4167
#5  0x5679e699 in test_dir_download_status_random_backoff (arg=0x0)
    at src/test/test_dir.c:4196
#6  0x56a519dc in testcase_run_bare_ (
    testcase=testcase@entry=0x5724e0c0 <dir_tests+640>)
    at src/ext/tinytest.c:106
#7  0x56a51de1 in testcase_run_one (group=<optimized out>, 
    group@entry=0x572346b0 <testgroups+208>, testcase=<optimized out>, 
    testcase@entry=0x5724e0c0 <dir_tests+640>) at src/ext/tinytest.c:253
#8  0x56a532c4 in tinytest_main (c=2, v=0xffffd764, 
    groups=0x572345e0 <testgroups>) at src/ext/tinytest.c:435
#9  0x5661eede in main (c=2, v=<optimized out>)
    at src/test/testing_common.c:319

I think this has something to do with our fixes for #17750 or #20534, but I'm not certain.

I have a patch that fixes this issue for me.

Child Tickets

Change History (5)

comment:1 Changed 3 years ago by nickm

See my branch bug22924 against master. I haven't figured out when this bug was first introduced, though.

comment:2 in reply to:  1 Changed 3 years ago by teor

Status: newneeds_revision
Version: Tor:

Replying to nickm:

See my branch bug22924 against master. I haven't figured out when this bug was first introduced, though.

It's been there since test_dir_download_status_random_backoff() was introduced in commit 1553512 in tor-, but we didn't find it until #17750, because the pre-#17750 unit tests used limited ranges.

I tested the patch on i386 and x86_64 macOS, and it passes on both.

comment:3 Changed 3 years ago by nickm

Okay. So it seems to me I should cherry-pick this onto the branch for 17750, in case we decide to backport that any more.

comment:4 Changed 3 years ago by nickm

I've added it to bug17750_029_squashed, and re-merged that into master. I'll make a note on #17750. Thanks for the review!

comment:5 Changed 3 years ago by nickm

Resolution: fixed
Status: needs_revisionclosed
Note: See TracTickets for help on using tickets.