Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#19400 closed defect (fixed)

[Asan] Crash in js::AsmJSModule::deserialize / DeserializeSig

Reported by: cypherpunks Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Critical Keywords: tbb-crash, TorBrowserTeam201606, tbb-gitian
Cc: virtualritz, mcs, brade, arthuredelstein, torbacchi, boklm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Steps to reproduce:

  1. Open current tor browser alpha, hardened (6.5a1)
  2. surf on facebookcorewwwi.onion
  3. click somewhere to start composing a message
  4. as soon as you can, try to type (not sure this is required)

What happens:
Tor browser crashes.

Date Time [notice] Bootstrapped 100%: Done
Date Time [notice] New control connection opened from 127.0.0.1.
Date Time [notice] New control connection opened from 127.0.0.1.
Time	addons.productaddons	ERROR	Request failed certificate checks: [Exception... "SSL is required and URI scheme is not https."  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: resource://gre/modules/CertUtils.jsm :: checkCert :: line 145"  data: no]
=================================================================
==5252==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f6dfe8c6000 at pc 0x7f6e4c3f2605 bp 0x7f6e009f23f0 sp 0x7f6e009f1ba0
READ of size 9437184 at 0x7f6dfe8c6000 thread T59 (DOM Worker)
ASAN:SIGSEGV
==5252==AddressSanitizer: while reporting a bug found another one. Ignoring.
Date Time [notice] Owning controller connection has closed -- exiting now.
Date Time [notice] Catching signal TERM, exiting cleanly.

Child Tickets

Attachments (2)

tor-browser-hardened_gdb_reduced.log (272.2 KB) - added by cypherpunks 3 years ago.
A backtrace from gdb attached to tor browser hardened
mega_sigsev (13.7 KB) - added by gk 3 years ago.

Download all attachments as: .zip

Change History (40)

Changed 3 years ago by cypherpunks

A backtrace from gdb attached to tor browser hardened

comment:1 Changed 3 years ago by gk

Keywords: TorBrowserTeam201606 added
Priority: MediumVery High
Severity: MajorCritical

comment:2 Changed 3 years ago by cypherpunks

Priority: Very HighMedium
Severity: CriticalMajor

Bug #19337 might be related.

comment:3 Changed 3 years ago by cypherpunks

Priority: MediumVery High
Severity: MajorCritical

Sorry, I accidentially broke priority/severity.

comment:4 Changed 3 years ago by gk

Cc: virtualritz mcs brade arthuredelstein added
Status: newneeds_information

It seems I can't get my hand on a Facebook account which is kind of required to reproduce this bug. If anybody has some (dormant) one for testing please let me know (gk [@] torproject.org 35CD 74C2 4A9B 15A1 9E1A 81A1 9437 3AA9 4B7C 3223).

That said there are still things that can be done to try finding more out about this bug.

1) Does saving the whole website locally and using that one (via file:///) crash as well? If so, could I get a copy of that website (see above for contact details)?
2) Does 6.0a5-hardened (or 6.0) crash as well?
3) If you are adventurous, testing a vanilla Firefox ASan build (https://ftp.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-linux64-asan/1450090940/) would be pretty interesting. Pointing it to your local tor should be all that is needed to reach facebookcorewwwi.onion.

comment:5 in reply to:  4 Changed 3 years ago by cypherpunks

Replying to gk:

2) Does 6.0a5-hardened (or 6.0) crash as well?

No, it doesn't. Crash happens just since the latest update from 6.0a5-hardened to 6.5a1-hardened or 6.0.1 (see bug #19337).

comment:6 in reply to:  4 Changed 3 years ago by cypherpunks

Replying to gk:

3) If you are adventurous, testing a vanilla Firefox ASan build (https://ftp.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-linux64-asan/1450090940/) would be pretty interesting. Pointing it to your local tor should be all that is needed to reach facebookcorewwwi.onion.

That doesn't reproduce the crash. But this FF version is ~6 months old, the change was probably introduced between FF 45 and FF ESR 45.2 (see comment #5)

comment:7 in reply to:  4 ; Changed 3 years ago by cypherpunks

Replying to gk:

1) Does saving the whole website locally and using that one (via file:///) crash as well? If so, could I get a copy of that website (see above for contact details)?

No, it doesn't crash. I used a vanilla FF 47 profile to save the page and opened it in TBB 6.5a1-hardened and it didn't crash.

comment:8 Changed 3 years ago by gk

Okay, thanks so much for narrowing this down. As far as I can see, there are three possible sources of the crash:

1) One of our (crash) fixes in 6.0.1 is causing this.
2) It is a bug in Mozilla's code itself.
3) It is a bug caused by new Mozilla code interfering badly with one of our patches.

I've uploaded a test build trying to rule out 1). It only omits the fixes for #19212 and #19187 and is the code otherwise the same as the branch used for Tor Browser 6.0.1.

https://people.torproject.org/~gk/testbuilds/tor-browser-linux64-bug19400-hardened_ALL.tar.xz
https://people.torproject.org/~gk/testbuilds/tor-browser-linux64-bug19400-hardened_ALL.tar.xz.asc

Does this still crash for you?

comment:9 in reply to:  8 Changed 3 years ago by gk

Replying to gk:

Okay, thanks so much for narrowing this down. As far as I can see, there are three possible sources of the crash:

1) One of our (crash) fixes in 6.0.1 is causing this.
2) It is a bug in Mozilla's code itself.
3) It is a bug caused by new Mozilla code interfering badly with one of our patches.

I've uploaded a test build trying to rule out 1). It only omits the fixes for #19212 and #19187 and is the code otherwise the same as the branch used for Tor Browser 6.0.1.

https://people.torproject.org/~gk/testbuilds/tor-browser-linux64-bug19400-hardened_ALL.tar.xz
https://people.torproject.org/~gk/testbuilds/tor-browser-linux64-bug19400-hardened_ALL.tar.xz.asc

Does this still crash for you?

It actually only omits the fix for #19187. The other build omitting #19212 is coming shortly.

comment:11 Changed 3 years ago by cypherpunks

The build in comment #10 doesn't crash. Looks like you found the cause.

comment:12 in reply to:  11 Changed 3 years ago by gk

Replying to cypherpunks:

The build in comment #10 doesn't crash. Looks like you found the cause.

Thanks. Could you test the build in comment:8 as well? That way we can be sure which one of the two fixes we shipped with 6.0.1 is causing this.

comment:13 Changed 3 years ago by cypherpunks

The build in comment #8 doesn't crash either.

comment:14 Changed 3 years ago by gk

Thanks! I've confirmed by manual testing that the build in comment:8 only omits the patch for #19187 and the build in comment:10 omits both. This seems to indicate that the backport of the ASan crash fix is somehow involved which does not make much sense to me. At least according to Mozilla this issue should only affect chrome code. Hm.

comment:15 Changed 3 years ago by gk

Hi cypherpunk: could you test one final build (I think we are pretty close to getting this reproduced) and see whether it is crashing for you:

https://people.torproject.org/~gk/testbuilds/tor-browser-linux64-bug19400_v3-hardened_ALL.tar.xz
https://people.torproject.org/~gk/testbuilds/tor-browser-linux64-bug19400_v3-hardened_ALL.tar.xz.asc

Thanks!

comment:16 Changed 3 years ago by gk

Okay, I think I found out some important things:

0) I assume the crashes on mega.nz some of our users observe are caused by the same underlying flaw. I attach a stacktrace from a mega.nz related crash that should be similar enough to justify treating it as the same bug.

1) The first crucial bit that was missing so far was that updating must be involved to reproduce the problem. I.e. I am pretty sure that using a clean, new 6.0.1 or 6.5a1 is working fine (can you confirm this, cypherpunk?). That would explain our problems reproducing the crash I guess.

2) The second crucial bit is that one must have visited e.g. mega.nz once before the update (I guess this applies to Facebook as well but I don't have an account to verify this). "Ideally", you have mega.nz open, apply your update and visit mega.nz again and it crashes.

3) The problem is confined to the Tor Browser profile. More specifically, for some reason there is a https+++mega.nz folder in profile.default/storage/temporary that contains binary asmjs/moduleN files which are different between a clean new profile used to visit mega.nz once and a profile that contains them after the update. Not sure whether that difference is enough to explain the crashes (probably not) but removing https+++mega.nz solves the problem for me.

4) This is no issue with a vanilla Firefox. I tried applying my STR to Firefox 45.1.1esr/45.2.0esr and did not hit this problem.

Things I still don't understand are

a) What role exactly plays the updater here?
b) How can it be that these asmjs modules are saved to disk given that we are in PBM?
c) Which of our patches is actually causing this problem given 4)?

Changed 3 years ago by gk

Attachment: mega_sigsev added

comment:17 Changed 3 years ago by gk

Cc: torbacchi added
Keywords: tbb-6.0-issues added
Status: needs_informationassigned

Resolved #19405 as duplicate.

comment:18 in reply to:  16 ; Changed 3 years ago by mcs

Replying to gk:

...
b) How can it be that these asmjs modules are saved to disk given that we are in PBM?

This is https://bugzilla.mozilla.org/show_bug.cgi?id=1047105 (which seems important but hasn't received much attention so far).

comment:19 in reply to:  16 Changed 3 years ago by mcs

Replying to gk:

a) What role exactly plays the updater here?

Running the updater is not required. I was able to reproduce the crash by first visiting Facebook using Tor Browser 6.0 and then running a clean copy of 6.0.1 (I installed it next to 6.0 so that the TorBrowser-Data directory was shared). After switching back to 6.0, things are fine again (no crash).

comment:20 in reply to:  15 ; Changed 3 years ago by cypherpunks

Replying to gk:

Hi cypherpunk: could you test one final build (I think we are pretty close to getting this reproduced) and see whether it is crashing for you:

https://people.torproject.org/~gk/testbuilds/tor-browser-linux64-bug19400_v3-hardened_ALL.tar.xz
https://people.torproject.org/~gk/testbuilds/tor-browser-linux64-bug19400_v3-hardened_ALL.tar.xz.asc

This build looks fine, can't reproduce the crash with it.

comment:21 in reply to:  20 Changed 3 years ago by gk

Replying to cypherpunks:

Replying to gk:

Hi cypherpunk: could you test one final build (I think we are pretty close to getting this reproduced) and see whether it is crashing for you:

https://people.torproject.org/~gk/testbuilds/tor-browser-linux64-bug19400_v3-hardened_ALL.tar.xz
https://people.torproject.org/~gk/testbuilds/tor-browser-linux64-bug19400_v3-hardened_ALL.tar.xz.asc

This build looks fine, can't reproduce the crash with it.

Thanks. This is using exactly the same branch as we have in 6.0.1. Thus, you can work around that crash for now by either installing a fresh new Tor Browser, or, if you are adventurous, just deleting the asmjs related files in your profile directory (see comment:16 and assuming Facebook is caching such stuff too) should help, too.

comment:22 Changed 3 years ago by cypherpunks

My crashing 6.5a1-hardened which has been updated from previous versions by updater has a profile.default/storage/temporary/https+++www.facebookcorewwwi.onion/ folder. After deleting it, the crash from this issue description is gone. This error message still exists:

addons.productaddons	ERROR	Request failed certificate checks: [Exception... "SSL is required and URI scheme is not https."  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: resource://gre/modules/CertUtils.jsm :: checkCert :: line 145"  data: no]

comment:23 Changed 3 years ago by cypherpunks

Following these steps I get a different crasher:

  1. Installing a vanilla 6.0a5-hardened
  2. visiting facebook
  3. update to 6.5a1-hardened
  4. visit facebook again, type in different comment fields

I get this now (not immediately though):

1465932200900	addons.productaddons	ERROR	Request failed certificate checks: [Exception... "SSL is required and URI scheme is not https."  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: resource://gre/modules/CertUtils.jsm :: checkCert :: line 145"  data: no]
=================================================================
==23172==ERROR: AddressSanitizer: stack-buffer-underflow on address 0x7f2e552749a0 at pc 0x7f2ea6de6605 bp 0x7f2e585a53f0 sp 0x7f2e585a4ba0
READ of size 9437184 at 0x7f2e552749a0 thread T61 (DOM Worker)
ASAN:SIGSEGV
==23172==AddressSanitizer: while reporting a bug found another one. Ignoring.
  1. removing the profile.default/storage/temporary/https+++www.facebookcorewwwi.onion/
  2. browse facebookcorewwwi.onion again, type some comments, etc: Crasher is gone.

I'll post a backtrace once I can (may take a day)

comment:24 Changed 3 years ago by mcs

Here are some additional things that Kathy and I learned today:

  • With Facebook, the crash is easy to reproduce after a 6.0 -> 6.0.1 transition and after a 6.0a5 -> 6.5a1 transition, both on 64 bit Linux and on Mac OS (we did not try on Windows).
  • After we built our own 6.5a1 Mac package using the Firefox build procedure (no gitian), we could not reproduce the crash. This was built from commit b60b8871fa08feaaca24bcf6dff43df0cd1c5f29.
  • When we did the same on Linux, we could not reproduce the crash either.
  • With our own 6.5a1 Linux build that was made using the gitian-based build process, the crash occurs (no surprise there).
  • The crash also occurs on Linux with a gitian-based build that has no Tor Browser patches at all (commit eba381b5a1d26f1c5d5ba51c67117cae985680c4).

The last thing makes me think we should be able to reproduce this with Firefox, but looking at the evidence as a whole I am starting to suspect a compiler bug. But I am out of time and energy for today.

Last edited 3 years ago by mcs (previous) (diff)

comment:25 in reply to:  18 Changed 3 years ago by gk

Replying to mcs:

Replying to gk:

...
b) How can it be that these asmjs modules are saved to disk given that we are in PBM?

This is https://bugzilla.mozilla.org/show_bug.cgi?id=1047105 (which seems important but hasn't received much attention so far).

This is #19417 on our side now.

comment:26 in reply to:  7 ; Changed 3 years ago by Chopchipcyps

Replying to cypherpunks:

Replying to gk:

1) Does saving the whole website locally and using that one (via file:///) crash as well? If so, could I get a copy of that website (see above for contact details)?

No, it doesn't crash. I used a vanilla FF 47 profile to save the page and opened it in TBB 6.5a1-hardened and it didn't crash.

Hope this is in the right place:
Can confirm here - running ML - tried reinstalling, then reverting back to 6 - it does not matter. The moment I begin to type something or interact with FB normal site, Tor crashes.

I can only run FB through Tor if it is in mobile site.

CRASH REPORT:
Crashed Thread: 61 DOM Worker

Exception Type: EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x0000000132be6000

VM Regions Near 0x132be6000:

mapped file 0000000132b00000-0000000132be6000 [ 920K] r--/rwx SM=COW /Users/USER/Library/Application Support/TorBrowser-Data/*

-->

VM_ALLOCATE 0000000132c00000-0000000132d00000 [ 1024K] rw-/rwx SM=PRV

CONSOLE:
15/06/2016 11:21:40.058 com.apple.launchd.peruser.501[209]: ([0x0-0x109008f].org.mozilla.tor browser[25824]) Job appears to have crashed: Segmentation fault: 11

comment:27 Changed 3 years ago by bugzilla

Why don't you offer the Medium-Low setting (at min) of the Security Slider as the easiest workaround of this problem? (Your solutions lead to tbb-disk-leak)
Proposal: include disabled ASM.JS into the Low setting (at least until it stops to violate PBM #19417).

comment:28 in reply to:  26 ; Changed 3 years ago by mcs

Replying to Chopchipcyps:

...
Hope this is in the right place:
Can confirm here - running ML - tried reinstalling, then reverting back to 6 - it does not matter. The moment I begin to type something or interact with FB normal site, Tor crashes.

I can only run FB through Tor if it is in mobile site.

To work around this problem, make a clean start by removing your entire TorBrowser-Data directory (in your case, it is located at /Users/USER/Library/Application Support/TorBrowser-Data). Or you can remove the cached asmjs files which you can find under .../TorBrowser-Data/Browser/*.default/storage/temporary/ (it should be OK to remove everything under the temporary directory).

comment:29 in reply to:  24 Changed 3 years ago by gk

Replying to mcs:

  • The crash also occurs on Linux with a gitian-based build that has no Tor Browser patches at all (commit eba381b5a1d26f1c5d5ba51c67117cae985680c4).

The last thing makes me think we should be able to reproduce this with Firefox, but looking at the evidence as a whole I am starting to suspect a compiler bug. But I am out of time and energy for today.

Thanks, that helped. I tried different things today to narrow this further down and I finally got a gitian build going that is not crashing. I hope to pin down the culprit tomorrow and maybe that will be the time for a proper fix even.

comment:30 Changed 3 years ago by mcs

Kathy and I are waiting for our own gitian build to finish before we can be sure (and you may already know some of this), but we believe that a not-so-beautiful combination of factors led to this crash:

1) This Firefox change that was made for 45.2.0 ESR: https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-45.2.0esr-6.5-1&id=fcb31773712f1e2adce790771f7978ba30056645

2) The fact that our build ID does not change between releases. The asmjscache code includes the build ID in serialized files and uses it to determine if cached files should be used or not (because serialized data structure sizes may change between browser releases). I would feel better if the serialized data structures had built-in bounds checking, e.g., by using a <length><payload> format for everything, but they do not. For build ID usage, see:

https://gitweb.torproject.org/tor-browser.git/tree/dom/asmjscache/AsmJSCache.cpp?h=tor-browser-45.2.0esr-6.5-1#n113
https://gitweb.torproject.org/tor-browser.git/tree/dom/asmjscache/AsmJSCache.cpp?h=tor-browser-45.2.0esr-6.5-1#n1697
https://gitweb.torproject.org/tor-browser.git/tree/js/src/asmjs/AsmJSModule.cpp?h=tor-browser-45.2.0esr-6.5-1#n2000
https://gitweb.torproject.org/tor-browser.git/tree/js/src/asmjs/AsmJSModule.cpp?h=tor-browser-45.2.0esr-6.5-1#n2317

The build ID issue also explains why our non-gitian builds do not crash (those builds use a build ID that is based on the actual build date like Firefox does).

If we do not want to disable the asmjscache unconditionally, then we will have to come up with a way to return a value from dom/asmjscache/AsmJSCache.cpp's GetBuildId() method that is unique for each of our releases.

comment:31 Changed 3 years ago by mcs

It looks like Mozilla is starting to using build ID for more things like this too, e.g., see https://bugzilla.mozilla.org/show_bug.cgi?id=1153978

comment:32 in reply to:  28 Changed 3 years ago by Chopchipcyps

Replying to mcs:

Replying to Chopchipcyps:

...
Hope this is in the right place:
Can confirm here - running ML - tried reinstalling, then reverting back to 6 - it does not matter. The moment I begin to type something or interact with FB normal site, Tor crashes.

I can only run FB through Tor if it is in mobile site.

To work around this problem, make a clean start by removing your entire TorBrowser-Data directory (in your case, it is located at /Users/USER/Library/Application Support/TorBrowser-Data). Or you can remove the cached asmjs files which you can find under .../TorBrowser-Data/Browser/*.default/storage/temporary/ (it should be OK to remove everything under the temporary directory).

I'll try that now - thanks.

comment:33 in reply to:  30 Changed 3 years ago by gk

Cc: boklm added
Keywords: tbb-gitian added; tbb-6.0-issues removed

Replying to mcs:

Kathy and I are waiting for our own gitian build to finish before we can be sure (and you may already know some of this), but we believe that a not-so-beautiful combination of factors led to this crash:

1) This Firefox change that was made for 45.2.0 ESR: https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-45.2.0esr-6.5-1&id=fcb31773712f1e2adce790771f7978ba30056645

2) The fact that our build ID does not change between releases. The asmjscache code includes the build ID in serialized files and uses it to determine if cached files should be used or not (because serialized data structure sizes may change between browser releases). I would feel better if the serialized data structures had built-in bounds checking, e.g., by using a <length><payload> format for everything, but they do not. For build ID usage, see:

https://gitweb.torproject.org/tor-browser.git/tree/dom/asmjscache/AsmJSCache.cpp?h=tor-browser-45.2.0esr-6.5-1#n113
https://gitweb.torproject.org/tor-browser.git/tree/dom/asmjscache/AsmJSCache.cpp?h=tor-browser-45.2.0esr-6.5-1#n1697
https://gitweb.torproject.org/tor-browser.git/tree/js/src/asmjs/AsmJSModule.cpp?h=tor-browser-45.2.0esr-6.5-1#n2000
https://gitweb.torproject.org/tor-browser.git/tree/js/src/asmjs/AsmJSModule.cpp?h=tor-browser-45.2.0esr-6.5-1#n2317

The build ID issue also explains why our non-gitian builds do not crash (those builds use a build ID that is based on the actual build date like Firefox does).

If we do not want to disable the asmjscache unconditionally, then we will have to come up with a way to return a value from dom/asmjscache/AsmJSCache.cpp's GetBuildId() method that is unique for each of our releases.

Thanks for this analysis! Things are starting to make sense. :) Your idea is actually #11506 which I did not find a good solution for while looking briefly at the options Gitian itself gives us.

I am inclined to stop using the asmjscache for now at least until #19417 is somehow solved. This would fix both issues for us. I wonder whether just backing out the patch in 1) would work as well which would then be another option.

comment:34 Changed 3 years ago by gk

Yes, backing out that patch seems to fix our problem, too. Oh, that irony: Mozilla was actually thinking of us while wondering whether to backport the compiler bug workaround...

comment:35 Changed 3 years ago by gk

Resolution: fixed
Status: assignedclosed

Backing out the compiler bug workaround in 2fbfb9d9af2553b35ec9231f3923b3be49ab2549 (tor-browser-45.2.0esr-6.5-1) and 6803b2f08165a02cac4621db10fc2e1d99b7b435 (tor-browser-45.2.0esr-6.0-1).

comment:36 Changed 3 years ago by cypherpunks

Why is there any cached state across browser launches in the first place?! It seems like this must be indicative of a bigger problem!

comment:37 in reply to:  36 Changed 3 years ago by mcs

Replying to cypherpunks:

Why is there any cached state across browser launches in the first place?! It seems like this must be indicative of a bigger problem!

Agreed. See #19417.

comment:38 Changed 3 years ago by mcs

Resolved #19657 as duplicate.

Note: See TracTickets for help on using tickets.