SIGSEGV on particular website with developer tools open

System: linux 64bit (arch linux), gnome3, Tor browser 6.0

Actions to reproduce:

1. Open F12 tools (Network tab if it matters)
2. Go to https://archive.org/details/a2_Zork_I_The_Great_Underground_Empire_1980_Infocom
3. Click on the play button.
4. After some downloads, the browser crashes.

I've tested this with both the security slider on low as on medium-high and that changes nothing. I went ahead and downloaded the debug systems and got a backtrace in gdb, which I'll include.

If there is anything else that I can help with, do let me know!

added component::applications/tor browser noscript owner::tbb-team priority::very high resolution::fixed severity::critical status::closed tbb-crash type::defect labels

Trac:
backtrace

backtrace from gdb

Confirmed on Win 7:

Faulting application name: firefox.exe, version: 45.1.1.0, time stamp: 0x00000000
Faulting module name: xul.dll, version: 45.1.1.0, time stamp: 0x00000000
Exception code: 0xc0000005
Fault offset: 0x025f8a22

A first look at it: this is due to one of our patches and it is already available on 6.0a5. Arthur/Mark/Kathy have any of you time to start working on this. I can continue tomorrow if you run out of time today.

Trac:
Keywords: N/A deleted, tbb-crash added
Severity: Normal to Critical
Priority: Medium to Very High
Cc: N/A to arthuredelstein, brade

Trac:
Cc: arthuredelstein, brade to arthuredelstein, brade, mcs

I can also reproduce this on Mac OS. I am out of time for today, but I will attach a debugger session log for a debug build, where an assertion fails inside the JS interpreter.

Trac:
19212-MacOS-Debug-CrashLog.txt

log from Mac OS debugger session

Two additional things:

• For me (on Mac OS), the crash only occurs when the developer tools are open and the Network tab is active (although the game does not load in other cases; I always see a "Failed to download game data" message).
• The crash still occurs when I set dom.indexedDB.enabled = true.

FWIW, I noticed that if I disable the NoScript extension (using the Add-Ons Manager) then the crash does not occur. On the other hand, if I use Firefox 45 ESR with NoScript installed, then I also do not see the crash.

Replying to mcs:

Two additional things:

• For me (on Mac OS), the crash only occurs when the developer tools are open and the Network tab is active (although the game does not load in other cases; I always see a "Failed to download game data" message).

That might be a related but different issue. I am still bisecting but I can see the "Failed to download game data" with the selected Network tab on https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-45.1.1esr-6.0-1&id=ff819d20657c0692e7005e11d04adffab4dd5efb while this is not crashing Tor Browser yet (while later commits are doing that).

https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-45.1.1esr-6.0-1&id=0b8b418640999e20218833e1ba188246bdd280f1 seems to be the culprit. I'll double check with two clobber builds in order to rule bisecting failures out. But I guess it is quite safe to work from the assumption that this patch is causing this.

Yes, confirmed. This is the problematic patch. Now, what went wrong...

Replying to gk:

Replying to mcs:

Two additional things:

• For me (on Mac OS), the crash only occurs when the developer tools are open and the Network tab is active (although the game does not load in other cases; I always see a "Failed to download game data" message).

That might be a related but different issue.

That's actually the case. This is due to us using private browsing mode and is reproducible in a vanilla Firefox as well.

So far: this is due to yield connectTimeline(); in https://mxr.mozilla.org/mozilla-esr45/source/devtools/client/netmonitor/netmonitor-controller.js#243

Okay, my time is up for now and I did not track it to a good next intermediate step down :(

Replying to gk:

So far: this is due to yield connectTimeline(); in https://mxr.mozilla.org/mozilla-esr45/source/devtools/client/netmonitor/netmonitor-controller.js#243

Good find. Avoiding connectTimeline() makes it do timeline events are not created, which avoids the codepath where Tor Browser crashes.

After trying for a while to find the root cause of this issue, Kathy and I started to look and see if Mozilla made any changes after ESR45 in code near the crash location. We found the following bug which looks like a duplicate of this ticket: https://bugzilla.mozilla.org/show_bug.cgi?id=1258535 The crash does disappear when I disable NoScript. Mozilla made two fixes, one to add more assertions and one to add an additional null pointer check: https://hg.mozilla.org/mozilla-central/rev/cd0123e0a09d https://hg.mozilla.org/mozilla-central/rev/029c36687f2f I am not convinced that they fixed the bug though. Applying their second patch did not fix this crash. Kathy and I are about out of time for today, but hopefully this gets us a little closer to a solution.

On second thought, this may not be an exact duplicate of the Mozilla bug because of our dependency on the #16528 (moved) patch. But it sure is close.

Replying to mcs:

On second thought, this may not be an exact duplicate of the Mozilla bug because of our dependency on the #16528 (moved) patch. But it sure is close.

It is not. I applied both fixes but the issue remains. I still don't understand why cx->compartment() returns a null pointer with our patch applied. One way to handle that could be to do simply return false in this case, quick and dirty. I am not that happy about this solution though, as this introcuces a second order patch which might have issues on its own.

Another way forward would be trying to understand why our patch is causing issues. I am still trying to wrap my head around the fact that basically just falling back to the PBM permission check is causing this. Why does commenting out those two code blocks cause our problem given that a normal Firefox in PBM does not care about those two code blocks either but does not crash anymore. (after bug 125835.

A third approach might be to understand what role NoScript plays. Why is it causing cx->compartment() to be null. We could ask Giorgio if he'd merge a patch for us.

I think I prefer option 2) right but time is up for now. I am back again in a couple of hours and I still have hope we can get that fixed in 6.0.1.

Here is another piece of data: Kathy and I can reproduce the crash after backing out the #16528 (moved) patch if we also flip dom.indexedDB.enabled to true (and stay in private browsing mode). In fact, we can also reproduce it with Firefox 46.0.1 in private browsing mode on Mac OS using a clean profile that has NoScript installed (with "Allow Scripts Globally" on).

We cannot reproduce it using Firefox Developer Edition (48.0a2 2016-06-02) using the same profile.

It seems there are more of these kinds of problems lurking in the Firefox code. See: https://bugzilla.mozilla.org/show_bug.cgi?id=1248948#c0 (I can reproduce that crash in TB 6.0).

Mozilla's fixes look like "bandaids" to me; the comments in the Mozilla bugs we have found so far do not indicate a deep understanding of the problem(s).

Isn't that https://bugzilla.mozilla.org/show_bug.cgi?id=1258535 (which got fixed in mozilla48)? (re comment:17)

Replying to gk:

Isn't that https://bugzilla.mozilla.org/show_bug.cgi?id=1258535 (which got fixed in mozilla48)? (re comment:17)

There is a different patch for it: https://hg.mozilla.org/mozilla-central/rev/1e5447cbadfa

Since Kathy and I are out of time for today, we created a workaround patch (in case that is the best thing anyone can come up with): https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug19212-01&id=d7f98ce46b9cc43bb4ec8482938cb04f6ca99e21

Arthur is fine with it, too, so taking for 6.0.1. In fact all other possible fixes would likely have been more complex making them more unsuitable for the stable. This is commit d7f98ce46b9cc43bb4ec8482938cb04f6ca99e21 on tor-browser-45.1.1esr-6.0-1.

Trac:
Status: new to closed
Resolution: N/A to fixed

what role NoScript plays. Why is it causing cx->compartment() to be null. We could ask Giorgio if he'd merge a patch for us.

Trac:
Keywords: N/A deleted, noscript added

closed

mentioned in issue #19400 (moved)

mentioned in issue #20680 (moved)

mentioned in issue #20680 (moved)

mentioned in issue #21712 (moved)

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

mentioned in issue tpo/applications/tor-browser#21712 (closed)