989992 - crashes in mozilla::dom::XULDocument::OnScriptCompileComplete or js::frontend::TokenStream::getTokenInternal with jid1-vW9nopuIAJiRHw@jetpack (SmileysWeLove) and tl_r@jetpack (TimeLineRemove) add-ons

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Robert Kaiser]

This bug was filed from the Socorro interface and is 
report bp-2a145635-0307-4c2e-bd00-5c2c62140331.
=============================================================

Both the signatures here were low volume but on 2014-03-29 they started spiking across channels (including 27.0.1 as well as 28.0 release and 29.0 beta, the signatures exist on Aurora 30 and Nightly 31 but are too low volume that a regression/spike can clearly be made out).

Correlations are pretty clear on all channels, see 28.0 for example:
[@ js::frontend::TokenStream::getTokenInternal(js::frontend::TokenStream::Modifier) ], Firefox 28.0 Windows NT:
97% (268/277) vs.   2% (1000/54830) jid1-vW9nopuIAJiRHw@jetpack

[@ mozilla::dom::XULDocument::OnScriptCompileComplete(JSScript*, tag_nsresult) ], Firefox 28.0 Windows NT:
100% (224/225) vs.   2% (1000/54830) jid1-vW9nopuIAJiRHw@jetpack

This means that all or almost all of the reports with those signatures have that add-on active while ~2% of the total reports do.

Either this add-on is triggering a crash in our code (which is somewhat assume given that it's about scripts) or the add-on code itself is at fault.

Even together, those crashes are not in the top 20 signatures on any channel, on 27, 28, and 29, the volume is around 80-100 crashes per 1M ADI for yesterday but significantly higher the day before.

Comment 1

[Robert Kaiser]

Oh, I forgot - stats and individual crash reports can be found at those URLs:
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Adom%3A%3AXULDocument%3A%3AOnScriptCompileComplete%28JSScript%2A%2C%20tag_nsresult%29
https://crash-stats.mozilla.com/report/list?signature=js%3A%3Afrontend%3A%3ATokenStream%3A%3AgetTokenInternal%28js%3A%3Afrontend%3A%3ATokenStream%3A%3AModifier%29

Comment 2

[Robert Kaiser]

<eviljeff> KaiRo: its addon 416610, fwiw
<KaiRo> eviljeff: what does that mean?
<eviljeff> the addon id on AMO.  I've got more details now...
<eviljeff> its called 'SmileysWeLove: use with Facebook, GMail, and more'
<eviljeff> its disabled on AMO now
<KaiRo> eviljeff: ah, that explains why crash rates are lower yesterday than the day before - why was it disabled?
<eviljeff> KaiRo: I can't see it was ever approved on AMO actually.  It kept getting rejected due to remote script insertion
<eviljeff> https://addons.mozilla.org/en-US/editors/review/smileys-we-love - jorgev - is there any other background?
<jorgev> no, it looks like it was an easy rejection
<jorgev> about a year ago, and never published on AMO
<KaiRo> jorgev: interesting that it caused a crash spike this weekend, then
<jorgev> it's likely it's being distributed with some other software

Comment 3

[Jorge Villalobos [:jorgev] (he/him)]

We can blocklist it if it turns out it is being silently installed. Kris, what do the stats say about this?

Comment 4

[Kris Maglione [:kmag]]

There's been a sharp rise in usage, all foreign installs, nearly all enabled, so yeah, it looks like this is being silently side-installed on a grand scale.

Comment 5

[Kris Maglione [:kmag]]

OK, I managed to find a download link. It does do a silent install from an external installer. It also asks to install a Conduit search-hijacker (https://people.mozilla.org/~kmaglione/images/d1dab73d46869385.png), so it's a pretty strong candidate for killing with fire.

Comment 6

[Jorge Villalobos [:jorgev] (he/him)]

The extension has been blocked, so that should probably eliminate the crashes. Kris, I think it would be a good idea to link to the installer or attach it so the crash can still be studied.

Comment 7

[Kris Maglione [:kmag]]

Attachment: [e-mail]

I got the installer from http://smileyswelove.com/Home/Download

The actual XPI is attached.

Comment 8

[[:philipp]]

the signature is still around and it appears as it is now in large part caused by the TimeLineRemove.Com 1.5.3 (tl_r@jetpack) extension:
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=js%3A%3Afrontend%3A%3ATokenStream%3A%3AgetTokenInternal%28js%3A%3Afrontend%3A%3ATokenStream%3A%3AModifier%29#tab-correlations

Comment 9

[Loic]

A user from the French community board reported same crashes with this signature. TimeLineRemove.Com 1.5.3 true (tl_r@jetpack) is installed in his profile.

Comment 10

[Loic]

CR: https://crash-stats.mozilla.com/report/index/bp-633a09c9-37dc-4bff-bc24-3a86a2140914

Comment 11

[[:philipp]]

Attachment: tl_r@jetpack 1.5.2

xpi sample of tl_r@jetpack version 1.5.2

Comment 12

[Robert Kaiser]

This is not 100% of those crashes, but a good part:
  js::frontend::TokenStream::getTokenInternal(js::frontend::TokenStream::Modifier)|EXCEPTION_ACCESS_VIOLATION_READ (362 crashes)
     88% (318/362) vs.   1% (513/84435) tl_r@jetpack
     20% (74/362) vs.   1% (1153/84435) {e4a8a97b-f2ed-450b-b12d-ee082ba24781} (Greasemonkey, https://addons.mozilla.org/addon/748)
     27% (97/362) vs.  15% (12984/84435) {d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d} (Adblock Plus, https://addons.mozilla.org/addon/1865)
     10% (35/362) vs.   0% (75/84435) [e-mail]


It sounds to me like different add-ons seem to hit on this. Are we sure it's not an actual bug in our code?

Comment 13

[[:philipp]]

would this be a candidate for blocklisting again?

Comment 14

[Jorge Villalobos [:jorgev] (he/him)]

Attachment: TimeLineRemoveFF.xpi

Kris, can you look into this new ID? The XPI can be downloaded from timelineremove.com, but I'm also attaching it in case it's useful.

Comment 15

[Benjamin Smedberg]

This looks like it's either one or two platform bugs to me. I'm not convinced that the two crash signatures here are at all related, though, and I wonder if we shouldn't separate the bugs.

Anyway, the stack traces from comment 8 are all truncated oddly, perhaps due to the JIT; can you pull one or a few and let us know more or all of the stack?

Comment 16

[(Away)]

mozjs!js::frontend::TokenStream::getTokenInternal
mozjs!js::frontend::Parser<js::frontend::FullParseHandler>::functionArguments
mozjs!js::frontend::Parser<js::frontend::FullParseHandler>::functionArgsAndBodyGeneric
mozjs!js::frontend::Parser<js::frontend::FullParseHandler>::standaloneLazyFunction
mozjs!js::frontend::CompileLazyFunction
mozjs!JSFunction::createScriptForLazilyInterpretedFunction
mozjs!JSFunction::getOrCreateScript
mozjs!js::LazyScript::functionDelazifying
mozjs!JSFunction::createScriptForLazilyInterpretedFunction
mozjs!JSFunction::getOrCreateScript
mozjs!js::Invoke
mozjs!js::Invoke
mozjs!JS::Call
xul!mozilla::dom::Function::Call
xul!mozilla::dom::Function::Call<nsCOMPtr<nsISupports> >
xul!nsGlobalWindow::RunTimeoutHandler
xul!nsGlobalWindow::RunTimeout
xul!nsGlobalWindow::TimerCallback
xul!nsTimerImpl::Fire
xul!nsTimerEvent::Run
xul!nsThread::ProcessNextEvent
xul!NS_ProcessNextEvent
xul!mozilla::ipc::MessagePump::Run
xul!MessageLoop::RunHandler
xul!MessageLoop::Run
xul!nsBaseAppShell::Run
xul!nsAppShell::Run
xul!XREMain::XRE_mainRun
xul!XREMain::XRE_main
xul!XRE_main
firefox!do_main
firefox!NS_internal_main
firefox!wmain
firefox!__tmainCRTStartup
kernel32!BaseThreadInitThunk
ntdll!__RtlUserThreadStart
ntdll!_RtlUserThreadStart

The other one is cut off even in my debugger. I assume the rest is just "the usual stuff below ProcessNextEvent". Could try to pull out more if you really want.

xul!mozilla::dom::XULDocument::OnScriptCompileComplete
xul!NotifyOffThreadScriptCompletedRunnable::Run
xul!nsThread::ProcessNextEvent

Comment 17

[Benjamin Smedberg]

ok looking at bp-7f6d6243-3df6-4fd3-9ff2-1b1ab2140911:

crash is at http://hg.mozilla.org/releases/mozilla-release/annotate/44234f451065/js/src/frontend/TokenStream.cpp#l1080

crash address is 0xffffffffffa12b7a

njn, is this likely to be an actual parser error, or some sort of GC/UAF error? Are there any interesting patterns of JS in the extension which might help provide clues to who might own this from an engineering perspective?

Comment 18

[Kris Maglione [:kmag]]

I looked at this the other day. I don't really have anything significant to add. It has about 25K users and seems to be legitimate. The privileged code is pretty trivial, and not anything I'd expect to cause a crash.

The page mod code is obfuscated, and a few thousand lines. It does seem to cause my deminifier a fair amount of trouble, but I can't say much beyond that.

Comment 19

[Nicholas Nethercote [inactive]]

> njn, is this likely to be an actual parser error, or some sort of GC/UAF
> error? Are there any interesting patterns of JS in the extension which might
> help provide clues to who might own this from an engineering perspective?

GC is unlikely to be involved. This is just tokenizing code. The crash occurs when we try to read a char, which happens only after checking that we haven't reached the end of the buffer.

I tried reproducing on the JS code within the TimeLineRemoveFF.xpi file, but failed. One of the files contains 573,765 chars of JS on a single line which is reasonable big but shouldn't really cause any problems.

If it really is a bug in the tokenizer then I'm a reasonable choice of owner. Steps to reproduce would be awfully helpful, of course.

Comment 20

[Nicholas Nethercote [inactive]]

I tried reproducing with the other two attached files, without success.

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

Closing because no crashes reported for 12 weeks.