Closed Bug 1028902 Opened 7 years ago Closed 5 years ago

crash in js::GetLengthProperty(JS::Value const&, JS::MutableHandle<JS::Value>)

Categories

(Core :: JavaScript Engine, defect)

33 Branch
x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox47 --- affected
firefox48 --- affected
firefox49 --- affected
firefox-esr45 --- affected

People

(Reporter: kairo, Unassigned)

References

Details

(Keywords: crash, topcrash-win)

Crash Data

This bug was filed from the Socorro interface and is 
report bp-08cca2cc-5913-4924-87b2-5a0432140623.
=============================================================

Top frames:
0 	mozjs.dll 	js::GetLengthProperty(JS::Value const &,JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter-inl.h
1 	mozjs.dll 	GetPropertyOperation 	js/src/vm/Interpreter.cpp
2 	mozjs.dll 	Interpret 	js/src/vm/Interpreter.cpp
3 	mozjs.dll 	js::RunScript(JSContext *,js::RunState &) 	js/src/vm/Interpreter.cpp
4 	mozjs.dll 	js::Invoke(JSContext *,JS::CallArgs,js::MaybeConstruct) 	js/src/vm/Interpreter.cpp

This started to happen on Nightly with the 6/20 build, across all Windows versions and on both 32bit and 64bit builds.
Based on this being the first nightly build it happens with, the regression range is http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f78e532e8a10&tochange=bdac18bd6c74

More reports at https://crash-stats.mozilla.com/report/list?signature=js%3A%3AGetLengthProperty%28JS%3A%3AValue%20const%26%2C%20JS%3A%3AMutableHandle%3CJS%3A%3AValue%3E%29
This bug might have the same root cause as bug 1028904, which started at the same time.
Version: 26 Branch → 33 Branch
Also note that https://crash-stats.mozilla.com/report/list?signature=EnterBaseline spiked at the same time as well.
So what we have is that lval.isString(), but we're crashing on lval.toString()->length(), with an access to memory address 0x4.

Sounds like toString() returned null?

Jan, could your latin1 changes in the range have anything to do with this?
Flags: needinfo?(jdemooij)
(In reply to Boris Zbarsky [:bz] from comment #3)
> Jan, could your latin1 changes in the range have anything to do with this?

My patches in that range are all pretty straight-forward. I double-checked them at least 5 times today :(
Flags: needinfo?(jdemooij)
I looked at some of the reports, and all of them have a weird addon installed. It often has a different name, so it's probably hard to use the Socorro correlation feature, but the version number is similar.

I can't find much about this addon, but some Googling shows that it's likely malware.

b1ac2ff7-8e51-4bb6-8bf8-87f1d567919a@4bb97481-aead-4c2e-a62b-e25e264651bb.com (version 0.94.109)
b1ac2ff7-8e51-4bb6-8bf8-87f1d567919a@4bb97481-aead-4c2e-a62b-e25e264651bb.com (version 0.94.83)
e2fd07a6-e282-4f2e-8965-85565fcb6384@b69158e6-3c3b-476c-9d98-ae5838c5b707.com (version 0.93.30)
39ed7c16-185d-4f88-b976-666d4928ba01@fe4550c1-7a4f-4a62-ad1c-45e0afdf81a4.com (version 0.94.83)
39ed7c16-185d-4f88-b976-666d4928ba01@fe4550c1-7a4f-4a62-ad1c-45e0afdf81a4.com (version 0.94.83)
fca3238e-0f52-4634-8e93-c36d211b2ea9@c1c012cf-93b0-488e-a2c5-453d23bec199.com (version 0.94.81)
508d4e2f-a469-421d-a294-135dbb84fe1b@f7b17943-cc9e-4d4a-b223-0bd1e7cfc871.com (version 0.94.39)
2eb528f3-950d-48a3-be4b-5d7de6c8331e@a41e199b-6ca4-4d23-ab87-73f2d1973314.com (version 0.94.308)
> This bug might have the same root cause as bug 1028904, which started at the
> same time.

Yeah, the setStringThis and the latest EnterBaseline ones have the same pattern of suspicious extensions.
I found the string "__CR_RESPONSE_READY" in a few stacks. Plus one similarly-versioned extension that forgot to disguise itself: crossriderapp14917@crossrider.com 	0.94.70

Looks like these extensions are created with the Crossrider framework. The framework itself could be legitimate, but searches for "__CR_RESPONSE_READY" and "CrossriderInitializerPlugin" lead to a lot of malware help forums.
Jan: if these crashes aren't related to your Latin1 string changes, what next actions to you suggest we take?

Can we find and install one of these malware addons using the Crossrider framework? Can we blocklist them?
Flags: needinfo?(jdemooij)
Jorge, any idea what we could do here?
Flags: needinfo?(jdemooij) → needinfo?(jorge)
Are those JS-only add-ons or are they binary? Esp. if they are JS-only, couldn't their code just be the trigger for an actual flaw on our side?
Judging by the generated IDs in comment #5, this is likely malware and will be very difficult to block. Kris, do you know if the Crossrider people keep track of the add-ons created with their framework? Could it be a bug in Crossrider itself?
Flags: needinfo?(jorge) → needinfo?(kmaglione+bmo)
I don't think it's necessarily all malware. Those add-ons seem to be mostly unrelated Crossrider add-ons, which means the issue is probably with the Crossrider framework.

I believe that Crossrider add-ons are mostly generated from a web IDE, and the IDs are generated by the framework. As I recall, though, we've warned them in the past that the IDs would need to be changed to indicate the source of the add-on.

We should probably just block all IDs using this format, and give the Crossrider devs a short window to fix whatever the issue in the framework is for the rest. I'd actually be completely happy to block the framework entirely until the developers come up with a version that passes AMO review. It's caused serious issues in the past, and I very much doubt that many of the add-ons in the wild provide any real value to our users.
Flags: needinfo?(kmaglione+bmo)
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #10)
> Are those JS-only add-ons or are they binary? Esp. if they are JS-only,
> couldn't their code just be the trigger for an actual flaw on our side?

It's possible, but since the crashes *only* happen with those addons and the fuzzers etc also haven't found anything, there's not much we can do.

(In reply to Kris Maglione [:kmag] from comment #12)
> We should probably just block all IDs using this format, and give the
> Crossrider devs a short window to fix whatever the issue in the framework is
> for the rest. I'd actually be completely happy to block the framework
> entirely until the developers come up with a version that passes AMO review.
> It's caused serious issues in the past, and I very much doubt that many of
> the add-ons in the wild provide any real value to our users.

That'd be great, to stop the crashes.
Blocks: 1036184
The crashes have gone down to a large degree about a week ago, but the remaining crashes are still one of the top 3 crash signatures on Nightly at this time.
Right now (yesterday), those addons and versions show up with this crash in Firefox Nightly 33.0a1:

048da175-3ee8-49e5-9d6f-2feb4d4793d5@3f15bd8f-93f6-4d68-a7c5-ae4f792d6bd4.com
 - 0.94.61, 0.94.63, 0.95.90
2eb528f3-950d-48a3-be4b-5d7de6c8331e@a41e199b-6ca4-4d23-ab87-73f2d1973314.com
 - 0.93.236, 0.94.253, 0.94.310, 0.94.312, 0.94.314, 0.95.324
5a6bf058-b978-4b84-a2ec-6f5462cfccb2@10120365-d3c0-4ec9-8624-5fac2592d0df.com
 - 0.94.52, 0.94.76, 0.94.77, 0.95.89
55d597b4-643f-421e-b007-26a68e26903b@a62d99f0-1402-44d5-8671-7a618c9c4868.com
 - 0.94.113, 0.94.118, 0.94.119, 0.95.120
sitematcher@sitematcher.com
 - 1.3, 1.5
39ed7c16-185d-4f88-b976-666d4928ba01@fe4550c1-7a4f-4a62-ad1c-45e0afdf81a4.com
 - 0.94.106, 0.95.129
143f44cf-d99c-4e45-8cd9-ef929de77aa8@bdbf6038-0097-480c-8d8e-fc48e28131a8.com
 - 0.94.67, 0.95.74

A Crossrider dev says in bug 1036184 that they did fix something - still, it's not clear to me if they are triggering a bug on our side (which we should fix) or if it's actually their bug.
Crash Signature: [@ js::GetLengthProperty(JS::Value const&, JS::MutableHandle<JS::Value>)] → [@ js::GetLengthProperty(JS::Value const&, JS::MutableHandle<JS::Value>)] [@ js::GetLengthProperty]
There are only 4 examples of this signature for a recent version on crash-stats for the past week, so I think this and bug 1028904 can be closed.

https://crash-stats.mozilla.com/signature/?signature=js%3A%3AGetLengthProperty
Blocks: 1028904
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
Crash volume for signature 'js::GetLengthProperty':
 - nightly (version 50): 0 crash from 2016-06-06.
 - aurora  (version 49): 0 crash from 2016-06-07.
 - beta    (version 48): 17 crashes from 2016-06-06.
 - release (version 47): 31 crashes from 2016-05-31.
 - esr     (version 45): 9 crashes from 2016-04-07.

Crash volume on the last weeks:
             Week N-1   Week N-2   Week N-3   Week N-4   Week N-5   Week N-6   Week N-7
 - nightly          0          0          0          0          0          0          0
 - aurora           0          0          0          0          0          0          0
 - beta             3          5          2          2          2          1          1
 - release          4          8          7          5          1          3          2
 - esr              0          0          0          0          6          1          1

Affected platforms: Windows, Mac OS X, Linux
Crash volume for signature 'js::GetLengthProperty':
 - nightly (version 50): 0 crashes from 2016-06-06.
 - aurora  (version 49): 1 crash from 2016-06-07.
 - beta    (version 48): 18 crashes from 2016-06-06.
 - release (version 47): 33 crashes from 2016-05-31.
 - esr     (version 45): 9 crashes from 2016-04-07.

Crash volume on the last weeks:
            W. N-1  W. N-2  W. N-3  W. N-4  W. N-5  W. N-6  W. N-7
 - nightly       0       0       0       0       0       0       0
 - aurora        0       0       0       0       0       0       0
 - beta          1       3       5       2       2       2       1
 - release       2       4       8       7       5       1       3
 - esr           0       0       0       0       0       6       1

Affected platforms: Windows, Mac OS X, Linux
You need to log in before you can comment on or make changes to this bug.