crash in js_NewStringCopyN

RESOLVED FIXED in Firefox 20

Status

()

--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Tobbi, Assigned: Benjamin)

Tracking

({crash, reproducible})

Trunk
mozilla21
crash, reproducible
Points:
---

Firefox Tracking Flags

(blocking-b2g:tef+, firefox18-, firefox19- wontfix, firefox20- verified, firefox21 verified, firefox-esr17-, b2g18 fixed, b2g18-v1.0.0 fixed)

Details

(crash signature)

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Steps to reproduce:
1. Install the JavaScript Deobfuscator Jetpack from https://addons.mozilla.org/en-US/firefox/addon/javascript-deobfuscator/?src=ss

2. Open the Deobfuscator from Tools > Web Developer > JavaScript Deobfuscator.

3. Open a new tab with grooveshark.com

Notice that it hangs the browser, then crashes.

Crash reports:
https://crash-stats.mozilla.com/report/index/bp-86c19f39-3eb7-4fad-b3de-634c22121230

https://crash-stats.mozilla.com/report/index/bp-26072cc0-c668-486f-ad3a-02f802121230
(Assignee)

Comment 1

6 years ago
You also need https://addons.mozilla.org/en-US/firefox/addon/grooveshark-no-ads/ to reproduce.
Assignee: general → benjamin
tracking-firefox18: --- → ?
tracking-firefox19: --- → ?
tracking-firefox20: --- → ?
tracking-firefox-esr17: --- → ?
OS: Windows 7 → All
Hardware: x86 → All
(Assignee)

Comment 2

6 years ago
Created attachment 696670 [details] [diff] [review]
make ScriptSource work if it is currently being compressed

The problem is that a ScriptSource whose sourc is being compressed can be reentered through the debugger's new script hook.

This patch makes ScriptSource's API behave correctly if it is used reentrantly.
Attachment #696670 - Flags: review?(jorendorff)
(In reply to :Benjamin Peterson from comment #2)
> Created attachment 696670 [details] [diff] [review]
> make ScriptSource work if it is currently being compressed
> 
> The problem is that a ScriptSource whose sourc is being compressed can be
> reentered through the debugger's new script hook.
> 
> This patch makes ScriptSource's API behave correctly if it is used
> reentrantly.

Considering we are so close to out Fx18 release, this is likely to go unfixed at this point.We have gone to build with all our beta's at this time.

Are there any other major website's being impacted here ?

Is this a Fx18 regression or if earlier versions of firefox are affected as well ? Do we know what bug regressed this ? Will be helpful to know this info to understand the need of tracking for future versions as well.
(Assignee)

Comment 4

6 years ago
(In reply to bhavana bajaj [:bajaj] from comment #3)
> Is this a Fx18 regression or if earlier versions of firefox are affected as
> well ? Do we know what bug regressed this ? Will be helpful to know this
> info to understand the need of tracking for future versions as well.

This is basically like bug 795104. It only manifests itself when the javascript debugger is being used in a certain way. The STR here rely on having two different addons installed, though in theory, I don't see why it couldn't happen with different addons that use the debugger. It certainly isn't common.

17 is affect which is why I asked for esr tracking. Maybe it could be considered for a 18 point release if that happens.

Comment 5

6 years ago
(In reply to :Benjamin Peterson from comment #4)
> (In reply to bhavana bajaj [:bajaj] from comment #3)
> > Is this a Fx18 regression or if earlier versions of firefox are affected as
> > well ? Do we know what bug regressed this ? Will be helpful to know this
> > info to understand the need of tracking for future versions as well.
> 
> This is basically like bug 795104. It only manifests itself when the
> javascript debugger is being used in a certain way.

Given that, we won't track for upcoming releases.
tracking-firefox18: ? → -
tracking-firefox19: ? → -
tracking-firefox20: ? → -
tracking-firefox-esr17: ? → -

Updated

6 years ago
Crash Signature: [@ memcpy | js_NewStringCopyN(JSContext*, wchar_t const*, unsigned int) ] [@ _VEC_memcpy | js_NewStringCopyN(JSContext*, wchar_t const*, unsigned int) ]
Keywords: reproducible
Summary: crash in [@ memcpy | js_NewStringCopyN(JSContext*, wchar_t const*, unsigned int) ] [@ _VEC_memcpy | js_NewStringCopyN(JSContext*, wchar_t const*, unsigned int) ] → crash in js_NewStringCopyN
Comment on attachment 696670 [details] [diff] [review]
make ScriptSource work if it is currently being compressed

Review of attachment 696670 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsapi-tests/testSourcePolicy.cpp
@@ +29,5 @@
> +newScriptHook(JSContext *cx, const char *fn, unsigned lineno,
> +              JSScript *script, JSFunction *fun, void *data)
> +{
> +    // Just need this not to fail.
> +    JS_StringEqualsAscii(cx, script->sourceData(cx), simpleSource, (JSBool *)data);

Actually we do check that the match succeeds, so the comment could be removed.

Might as well set *data to false if this JS_StringEqualsAscii call fails.

@@ +36,5 @@
> +BEGIN_TEST(testScriptSourceReentrant)
> +{
> +    JS::CompileOptions opts(cx);
> +    JSBool match = false;
> +    JS_SetNewScriptHook(JS_GetRuntime(cx), newScriptHook, &match);

Instead of `JS_GetRuntime(cx)`, write `rt`.
Attachment #696670 - Flags: review?(jorendorff) → review+
(Assignee)

Comment 10

6 years ago
Comment on attachment 696670 [details] [diff] [review]
make ScriptSource work if it is currently being compressed

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 761723
User impact if declined: Crashes with the specific set of addons mentioned in this bug. Possibly others, though others don't seem to have been reported.
Testing completed (on m-c, etc.): On m-i now
Risk to taking this patch (and alternatives if risky): Low-risk
String or UUID changes made by this patch: None
Attachment #696670 - Flags: approval-mozilla-beta?
Attachment #696670 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/038ff90d816b
https://hg.mozilla.org/mozilla-central/rev/488af66ab025
https://hg.mozilla.org/mozilla-central/rev/f735af7a57c3
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment on attachment 696670 [details] [diff] [review]
make ScriptSource work if it is currently being compressed

Approving for uplift to FF20. This still seems very edge casey, so please renominate for beta if we get user reports of this issue.
Attachment #696670 - Flags: approval-mozilla-beta?
Attachment #696670 - Flags: approval-mozilla-beta-
Attachment #696670 - Flags: approval-mozilla-aurora?
Attachment #696670 - Flags: approval-mozilla-aurora+

Updated

6 years ago
status-firefox20: --- → fixed
status-firefox21: --- → fixed
(Assignee)

Comment 14

6 years ago
Created attachment 708555 [details] [diff] [review]
backport to b2g18
(Assignee)

Comment 15

6 years ago
Comment on attachment 708555 [details] [diff] [review]
backport to b2g18

[Approval Request Comment]
Needed for tef+ bug 836515.
Attachment #708555 - Flags: approval-mozilla-b2g18?
If this is intended to land on v1_0_0 (which it presumably is if bug 836515 is tef+), this needs to be tef+ as well.
blocking-b2g: --- → tef?
Blocks: 836515
blocking-b2g: tef? → tef+
status-b2g18: --- → affected
status-b2g18-v1.0.0: --- → affected
Comment on attachment 708555 [details] [diff] [review]
backport to b2g18

Clearing the approval since this is now tef+ and you may uplift when ready.
Attachment #708555 - Flags: approval-mozilla-b2g18?
https://hg.mozilla.org/releases/mozilla-b2g18/rev/ced57d302db4
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_0/rev/2eb213a75f35
status-b2g18: affected → fixed
status-b2g18-v1.0.0: affected → fixed
status-firefox19: --- → wontfix
Verified that Firefox 20 beta 1 does not crash when following the STR from the Description and from Comment 1. 

Verified on Windows 7, Ubuntu 12.04 and Mac OS X 10.8:
Build ID: 20130220104816
Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20100101 Firefox/20.0

There are also no crash reports in Socorro related with the listed signatures on Firefox 20 beta 1.
status-firefox20: fixed → verified
QA Contact: simona.marcu
Verified as fixed on Firefox 21 beta 4. Also there are no crash reports in Socorro related with the listed signatures for Firefox 21 beta.

Verified on Windows 7, Ubuntu 12.10 and Mac OS X 10.8:
Build ID: 20130423212553
Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20100101 Firefox/21.0
Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20100101 Firefox/21.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:21.0) Gecko/20100101 Firefox/21.0
status-firefox21: fixed → verified
You need to log in before you can comment on or make changes to this bug.