TBBs with UBSan create lots of errors when running

added TorBrowserTeam201711 component::applications/tor browser owner::tbb-team priority::medium severity::normal status::assigned tbb-security type::defect labels

09:06 < GeKo> oh, btw. are you running (semi-)regular UBSan builds as well? I am 
              getting loads of "runtime error: load of value X, which is not a value 
              for type Y" errors.
09:06 < decoder> no, im not. the problem with ubsan is that most errors it reports 
                 are likely not going to be fixed by developers
09:07 < decoder> it's really hard to reason with devs about fixing those
09:07 < GeKo> ugh
09:07 < decoder> in some cases (e.g. signed overflows, float overflows, in layout/) 
                 developers said it's well known and wont be fixed
09:07 < decoder> the whole layout/ engine does signed overflows all the time and 
                 doesnt care about the result
09:08 < decoder> they say worst thing that can happen is wrong layouting
09:08 < GeKo> :(
09:09 < decoder> GeKo: ive found signed overflows before though with ubsan, that had 
                 a certain security impact
09:09 < decoder> problem is, with all those messages from layout/ and some other 
                 components that we need to ignore, we need a working blacklist 
                 mechanism
09:10 < decoder> so we can ignore those problems

Trac:
Keywords: N/A deleted, needs-triage added

Trac:
Keywords: needs-triage deleted, N/A added

Trac:
Keywords: N/A deleted, tbb-hardening added

Trac:
Parent: #10599 (moved) to N/A
Sponsor: N/A to N/A
Severity: N/A to Normal

Trac:
Keywords: tbb-hardening deleted, tbb-hardened added

Maybe, it's better to start using UBSan on FF's components step by step (JS, NSS, etc).

Trac:
Owner: erinn to tbb-team
Component: Applications/Tor bundles/installation to Applications/Tor Browser
Reviewer: N/A to N/A
Status: new to assigned

Replying to bugzilla:

Maybe, it's better to start using UBSan on FF's components step by step (JS, NSS, etc). I'd start with the image decoders. I know of at least one 0day being traded actively which is exploitable in Tor Browser in the highest security setting, and none of the people who I trade with are going to report it (neither can I). But UBSan is very likely to mitigate it, if trapped to ud2 with -fsanitize-undefined-trap-on-error, as well as others. After that, NSS is probably the most important, because it can't be turned off. JS has a huge surface area, but it can be disabled by the slider.

If I have free time, I'll try building FF with the image decoders using UBSan, but I'd really rather it if someone else who's already testing this stuff out do it since I've been very busy with other things (I might take a while).

Trac:
Cc: N/A to arthuredelstein

If Tor Browser is ever compiled with Clang (if that's even possible), the UBSan subset whitelist supports far more fine granularity, permitting the whitelisting of individual UBSan subsets within not only single files, but within single functions as well. This way, if a large file has several functions that violate, say, object-size, then those functions can avoid being instrumented, without having to disable it on the entire source file. More information about Clang's "special case list" is on https://clang.llvm.org/docs/SanitizerSpecialCaseList.html.

Has anyone started working on at least instrumenting individual FF components, as suggested above?

Replying to cypherpunks:

Has anyone started working on at least instrumenting individual FF components, as suggested above?

Kind of. Mozilla spend a considerable amount of person-time playing with UBSAN.

The conclusion was that some tests are valuable and should be used (bounds, pointer-overflow, vptr although this requires RTTI).

But that others (signed and unsigned overflow) caused a gratuitous amount of false positives (largely in the graphics and layout areas but in general all over the place) and it's infeasible to whitelist them all. We had someone spend a month on this and using his whitelist we brought the number of reports down from the hundred of thousands down to the mere thousands - but even then it was with a ton of effort and had a ton of effort to go.

So I think the path forward is to turn on UBSAN on the whole browser, run it through something like the web platform tests or Mozilla's usual unit tests, and slowly increase the number of UBSAN tests one by one. When we hit one that causes too many false positives, we turn it back off and investigate turning it on for an individual component (like image decoders.)

Also I would suggest the path forward for this is in Mozilla's court, rather than Tor's. Not that Tor has to wait for Mozilla, only that making use of Mozilla's infrastructure will make it considerably easier. Tor devs have access to that, and if any cypherpunks want access, I think the only thing needed is a few contributions* that I can point to and say "This person is doing good work, let's give them access to run their tests on our task runner".

* Contributions such as looking at and improving UBSAN? :)

Here's where our stuff is:

http://searchfox.org/mozilla-central/source/build/autoconf/sanitize.m4 - Here's where we have the mozconfig options for the sanitizers. We have the UBSAN integer one here. You can swap out integer for one of the other tests.
http://searchfox.org/mozilla-central/search?q=ubsan&path= - at the top you can find our blacklist

Replying to tom:

Replying to cypherpunks:

Has anyone started working on at least instrumenting individual FF components, as suggested above?

Kind of. Mozilla spend a considerable amount of person-time playing with UBSAN.

Wow! I had no idea so much work has already been put into this task! This will be very helpful.

The conclusion was that some tests are valuable and should be used (bounds, pointer-overflow, vptr although this requires RTTI).

But that others (signed and unsigned overflow) caused a gratuitous amount of false positives (largely in the graphics and layout areas but in general all over the place) and it's infeasible to whitelist them all. We had someone spend a month on this and using his whitelist we brought the number of reports down from the hundred of thousands down to the mere thousands - but even then it was with a ton of effort and had a ton of effort to go.

Unfortunately, those are some of the most important types of UB that must be prevented. An alternative (mutually exclusive due to incompatibilities with internal symbol names, or something of that sort), if suitable manpower is present, is to instrument important parts of FF with the PaX Size Overflow plugin (see https://forums.grsecurity.net/viewtopic.php?f=7&t=3043). It provides better protection than UBSAN for this specific issue.

So I think the path forward is to turn on UBSAN on the whole browser, run it through something like the web platform tests or Mozilla's usual unit tests, and slowly increase the number of UBSAN tests one by one. When we hit one that causes too many false positives, we turn it back off and investigate turning it on for an individual component (like image decoders.)

I had assumed that the amount of UB would be so great that it would be infeasible to do this in any reasonable amount of time. I still feel like instrumenting individual components of the browser would be easier.

Also I would suggest the path forward for this is in Mozilla's court, rather than Tor's. Not that Tor has to wait for Mozilla, only that making use of Mozilla's infrastructure will make it considerably easier. Tor devs have access to that, and if any cypherpunks want access, I think the only thing needed is a few contributions* that I can point to and say "This person is doing good work, let's give them access to run their tests on our task runner".

I tend to avoid Mozilla's ticket system due to their excessively bureaucratic nature, and their tendency to put security as a low priority. All my Firefox-related contributions have been made here (though admittedly I have made more contributions for Tor itself, and relatively few for Firefox).

Replying to cypherpunks:

Replying to tom:

The conclusion was that some tests are valuable and should be used (bounds, pointer-overflow, vptr although this requires RTTI).

But that others (signed and unsigned overflow) caused a gratuitous amount of false positives (largely in the graphics and layout areas but in general all over the place) and it's infeasible to whitelist them all. We had someone spend a month on this and using his whitelist we brought the number of reports down from the hundred of thousands down to the mere thousands - but even then it was with a ton of effort and had a ton of effort to go.

Unfortunately, those are some of the most important types of UB that must be prevented. An alternative (mutually exclusive due to incompatibilities with internal symbol names, or something of that sort), if suitable manpower is present, is to instrument important parts of FF with the PaX Size Overflow plugin (see https://forums.grsecurity.net/viewtopic.php?f=7&t=3043). It provides better protection than UBSAN for this specific issue.

Hmmmmm. I don't know I will have to investigate.

So I think the path forward is to turn on UBSAN on the whole browser, run it through something like the web platform tests or Mozilla's usual unit tests, and slowly increase the number of UBSAN tests one by one. When we hit one that causes too many false positives, we turn it back off and investigate turning it on for an individual component (like image decoders.)

I had assumed that the amount of UB would be so great that it would be infeasible to do this in any reasonable amount of time. I still feel like instrumenting individual components of the browser would be easier.

What do you mean by "amount of UBSAN"? There are some checks that should have basically no false-positives (like pointer overflow) - those should be feasible for whole-browser I think.

Instrumenting components with more verbose tests (like int overflows) is definitely valuable though!

Mind you, Mozilla's not going to ship Firefox with UBSAN enabled, we'll just run tests with it to catch issues. Maybe Tor would ship something with UBSAN (??) but maybe not since I don't think you can enable both ASAN and UBSAN.

Also I would suggest the path forward for this is in Mozilla's court, rather than Tor's. Not that Tor has to wait for Mozilla, only that making use of Mozilla's infrastructure will make it considerably easier. Tor devs have access to that, and if any cypherpunks want access, I think the only thing needed is a few contributions* that I can point to and say "This person is doing good work, let's give them access to run their tests on our task runner".

I tend to avoid Mozilla's ticket system due to their excessively bureaucratic nature, and their tendency to put security as a low priority. All my Firefox-related contributions have been made here (though admittedly I have made more contributions for Tor itself, and relatively few for Firefox).

Well, It's a big org, we're not all bad ;) But I hear you loud and clear. My main point was not "Try and get Mozilla to take your patches" but rather "You can almost certainly make use of Mozilla's infrastructure to do experimental runs and examine the output." For example, you could queue up 10 jobs that turn on UBSAN for 10 individual components, and run them all at once.

I started look into the ubsan errors by adding -fsanitize-undefined to a mozconfig in mozilla-central: https://github.com/arthuredelstein/tor-browser/commit/ubsan3

I pushed to the try server to run all unit tests and talos tests on linux, linux64 (debug and optimized) https://treeherder.mozilla.org/#/jobs?repo=try&revision=43f94f28e54232cd9fec8abb81b871121939aefd

Then I wrote scripts to download all logs files from this try server run, extract all "runtime errors" reported by ubsan in the logs, and then group the runtime errors by their location in the codebase.

https://github.com/arthuredelstein/firefox-ubsan-errors

In total there were some 170,000 runtime errors reported in the logs, produced by 367 specific locations in the codebase. (Some locations caused thousands of 'runtime error' messages each.) I generated a summary table that shows these locations and a representative error message. Here it is in a Google doc spreadsheet: https://docs.google.com/spreadsheets/d/1ISxhkwWVwa7HBVEd6gPTcynfMwaq-cmI_wQsiDZxLhc/edit?usp=sharing

And here is the same table in a tab-delimited text file: https://gist.github.com/arthuredelstein/a208b1d7334c9e1d669308b9cd06f96b

My next steps are to generate the same table for a clang -fsanitize=undefined build, and then start patching and/or whitelisting all functions in given category of ubsan error (such as integer overflow). If Mozilla can accept these patches, then I imagine we can turn on ubsan subflags in the mozilla-central debug builds and also turn them on by default in Tor Browser (alpha to begin with).

Trac:
Keywords: tbb-hardened deleted, TorBrowserTeam201708 added

Items for September 2017.

Trac:
Keywords: TorBrowserTeam201708 deleted, TorBrowserTeam201709 added

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 layout/* 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 patched 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.

Replying to tom:

What do you mean by "amount of UBSAN"? There are some checks that should have basically no false-positives (like pointer overflow) - those should be feasible for whole-browser I think.

Amount of UB (undefined behavior), not amount of UBSAN. And yes, some checks seem to very compatible with many programs (-fsanitize=object-size for example). Those should be the first tested, though instrumenting the image decoders as thoroughly as possible should be high priority.

Just a side-note, but -ftrapv and friends interfere with -fsanitize=signed-integer-overflow, such that the former disables the latter, even though it is easier to bypass. They should never be combined.

Instrumenting components with more verbose tests (like int overflows) is definitely valuable though!

Mind you, Mozilla's not going to ship Firefox with UBSAN enabled, we'll just run tests with it to catch issues. Maybe Tor would ship something with UBSAN (??) but maybe not since I don't think you can enable both ASAN and UBSAN.

You can enable both ASAN and UBSAN, but ASAN is not intended for use in production systems for security as the complex runtime can introduce vulnerabilities, and it's quite easy to bypass anyway considering it's looking for unintentional bugs that aren't trying to be stealthy. UBSAN on the other hand is fine for production use.

Well, It's a big org, we're not all bad ;) But I hear you loud and clear. My main point was not "Try and get Mozilla to take your patches" but rather "You can almost certainly make use of Mozilla's infrastructure to do experimental runs and examine the output." For example, you could queue up 10 jobs that turn on UBSAN for 10 individual components, and run them all at once.

I have a small cluster of computers which I can use to do the test myself, so long as I have the code for the unit tests. Getting trusted by Mozilla to the point where I can use their test infrastructure is not something I'm eager to commit to.

On a side-note, I've already tested Tor (not the browser) with -fsanitize=object-size, and it works perfectly.

Cool! And good work arthur. I signed up to tackle https://bugzilla.mozilla.org/show_bug.cgi?id=1402316 which should lay a framework for silencing individual errors we don't care about.

Replying to tom:

Cool! And good work arthur. I signed up to tackle https://bugzilla.mozilla.org/show_bug.cgi?id=1402316 which should lay a framework for silencing individual errors we don't care about.

Thanks, Tom. I commented on that ticket.

Items for October 2017

Trac:
Keywords: TorBrowserTeam201709 deleted, TorBrowserTeam201710 added

Moving tickets over to November.

Trac:
Keywords: TorBrowserTeam201710 deleted, TorBrowserTeam201711 added

As a status report: I have started work on trying to fix ubsan runtime errors in Firefox and Tor Browser. Here's my script to download all log files from the Mozilla task cluster, and extract, deduplicate, and categorize the ubsan errors from a try server run: https://github.com/arthuredelstein/firefox-ubsan-errors

I decided to first focus on he category of ubsan enum errors as a proof of principle. Here's the parent ticket: https://bugzilla.mozilla.org/show_bug.cgi?id=1404547

I opened and fixed the following tickets:

I also opened the following tickets:

https://bugzilla.mozilla.org/1404189 (fixed by André Bargull)
https://bugzilla.mozilla.org/1404220 (fixed by Paul Adenot)

There remain four more ubsan enum error locations. I expect two of these can be fixed, and the other two will need to be whitelisted using -fno-sanitize=enum. Then we can add -fsanitize=enum for the overall Firefox/TBB debug build.

Then the next steps will be to fix other types of ubsan runtime errors and then turn on aborts for them in the debug builds so that future ubsan errors can be discovered during automated test runs.

moved to tpo/applications/tor-browser#12418 (closed)

TBBs with UBSan create lots of errors when running

Child items 0

Activity