Re: [tor-bugs] #12418 [Applications/Tor Browser]: TBBs with UBSan create lots of errors when running

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#12418: TBBs with UBSan create lots of errors when running
------------------------------------------------+--------------------------
 Reporter:  gk                                  |          Owner:  tbb-team
     Type:  defect                              |         Status:  assigned
 Priority:  Medium                              |      Milestone:
Component:  Applications/Tor Browser            |        Version:
 Severity:  Normal                              |     Resolution:
 Keywords:  tbb-security, TorBrowserTeam201709  |  Actual Points:
Parent ID:                                      |         Points:
 Reviewer:                                      |        Sponsor:
------------------------------------------------+--------------------------

Comment (by arthuredelstein):

 Here is a summary of my recent efforts on this ticket.

 First, as a proof of principle, I patched most of the enums in mozilla-
 central that were showing ubsan errors. My branch is here:

 https://github.com/arthuredelstein/tor-browser/commits/ubsan-enum1

 There are a few further enum error locations I need to fix, and then I
 think I can propose adding `-fsanitize=enum` to Firefox debug builds and
 Tor Browser alpha builds.

 However, enum errors are not a likely source of security problems. Rather
 a much more dangerous undefined behavior caught by ubsan are the signed
 integer overflows.

 So I pushed a new try server run for linux on mozilla-central, with
 -fsanitize=signed-integer-overflow enabled:
 https://treeherder.mozilla.org/#/jobs?repo=try&revision=876a50a1c1122c02186f329f04d020aafa83c5f2

 There were a total of 15685 signed integer overflow events in 221
 locations:
 https://docs.google.com/spreadsheets/d/1yluXK-
 9V4_0C40WEZn3fS5LCOG_az_I6HcuptLBZ-qM/edit#gid=0 (Here's the raw file:
 https://gist.github.com/arthuredelstein/f711d25c997028a59414f42dc97ec7e3)

 I observed that 140 of the overflow sites (of 221) are located in
 `layout/*`. (These comprise nearly all of the undefined behaviors of any
 kind I found in comment:14. A further 15 are found in `gfx/cairo/*`, which
 is an external library that I suppose will need to be patched upstream.

 How do we deal with these errors? My currently thinking is, first we
 should suppress every one of these overflow warnings by patching or
 ignoring them. Then we can turn on
 {{{
 -fsanitize=signed-integer-overflow -fno-sanitize-recover=signed-integer-
 overflow
 }}}
 in the Mozilla debug build to prevent new overflow errors from being
 introduced. Later, as time allows, we can go back and try to stop ignoring
 errors in other sections of the code and fix them instead.

 So I decided to try to simplify the problem by temporarily suppressing
 errors in `layout/*` and `gfx/cairo/*`, by adding `-fno-sanitize=signed-
 integer-overflow` flags in their respective moz.build files. (I also
 patches a couple of locations in code to remove their overflows.) Here is
 the branch and the results:
 https://treeherder.mozilla.org/#/jobs?repo=try&revision=292ac14828e642d62e7fbc7fab1f719bc98c07bc

 There are about 45 remaining overflow locations in the code. Many of these
 seem to be signed integer additions where one of the addends has the value
 (2^31^ - `delta`), where `delta` is a small integer between 1 and 1000.
 The other addend is typically a small integer, but greater than `delta`,
 thus resulting in an overflow. So my next tasks are to work out (1) where
 some of these integers close to INT_MAX are coming from, and (2) how to
 handle that type of error.

 My hope is we can use Mozilla’s CheckedInt mechanism to handle these
 errors safely (instead of aborting via ubsan). Depending on the
 performance impact, we might need to write a `mozilla::CheckedInt`-based
 macro that only runs if we are using signed integer overflows.

 Other thoughts: in the future, we could investigate whether fuzzing the
 build will find more integer overflows that aren’t incidentally exposed by
 Mozilla’s regression tests. I believe some fuzzing of nightly builds is
 already being done, so turning on ubsan flags may already have interesting
 results.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/12418#comment:17>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online

_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs