Crash [@ nsImageLoadingContent::OnStartDecode]

VERIFIED FIXED in mozilla2.0b11

Status

()

defect
--
critical
VERIFIED FIXED
9 years ago
a month ago

People

(Reporter: jruderman, Assigned: jst)

Tracking

(Blocks 1 bug, {crash, regression, testcase})

Trunk
mozilla2.0b11
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+, blocking1.9.2 .17+, status1.9.2 .17-fixed, blocking1.9.1 .19+, status1.9.1 .19-fixed)

Details

(Whiteboard: [softblocker][fx4-fixed-bugday], crash signature)

Attachments

(6 attachments)

It looks like this does not crash on Firefox 3.6. Should that be a blocker?
blocking2.0: --- → ?
Keywords: regression
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
(Assignee)

Comment 2

9 years ago
Posted patch Fix.Splinter Review
This makes it so that only chrome can call methods on the interfaces that are not explicitly listed in classinfo for input elements.
Attachment #483355 - Flags: review?(jonas)
(Assignee)

Comment 3

9 years ago
Blocking.
blocking2.0: ? → final+
Comment on attachment 483355 [details] [diff] [review]
Fix.

If we're able to restrict access to Components.interfaces for FF4, then we should back this out.

r=me for now though.
Attachment #483355 - Flags: review?(jonas) → review+

Updated

9 years ago
Assignee: nobody → jst
Bug 605271 isn't going to make it.  We should land this on trunk.
Keywords: checkin-needed
I was thinking to push this but it doesn't pass try and fails a lot of tests. One of them is content/base/test/test_copyimage.html

For more information, you can see the try results here:
http://tbpl.mozilla.org/?tree=MozillaTry&rev=758292a1efa8
Keywords: checkin-needed
Whiteboard: [dont pass try]
(Assignee)

Updated

8 years ago
Whiteboard: [dont pass try] → [dont pass try], softblocker
(Assignee)

Updated

8 years ago
Whiteboard: [dont pass try], softblocker → [dont pass try][softblocker]
(Assignee)

Comment 8

8 years ago
So this got nasty. What happens here is that we have a bunch of code that ends up running from within untrusted JS, and that code calls into the image code to do whatever, and we check to see if the caller is trusted, but we find the untrusted JS code, and deny the C++ code from doing what it needs to do. This patch adds the (in)appropriate nsCxPusher's to push a null context where needed to hide the fact that this code was called from untrusted JS so that the C++ code can do what it needs to do. This really sucks, but I don't think we have a better option here.
Attachment #507741 - Flags: review?(jonas)
Comment on attachment 507741 [details] [diff] [review]
Fix test failures.

>@@ -145,7 +145,13 @@ nsSVGImageFrame::~nsSVGImageFrame()
>   if (mListener) {
>     nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(mContent);
>     if (imageLoader) {
>-      imageLoader->RemoveObserver(mListener);
>+      // Push a null JSContext on the stack so that code that runs
>+      // within the below code doesn't think it's being called by
>+      // JS. See bug 604262.
>+      nsCxPusher pusher;
>+      pusher.PushNull();
>+ 
>+     imageLoader->RemoveObserver(mListener);

There's a whitespace issue on the last line here.

r=me with that fixed.
Attachment #507741 - Flags: review?(jonas) → review+

Comment 10

8 years ago
This patch landed as:

http://hg.mozilla.org/mozilla-central/rev/4beb0e6ba654
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b11

Comment 11

8 years ago
Tested

Mozilla/5.0 (Windows NT 5.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b11 ID:20110203141415

does not crash when loading attachment.

4.0b10 did crash ! So it seems problem is solved.

Comment 12

8 years ago
Verified on Linux, too — with Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre  (crashed with 20110124030332)
Status: RESOLVED → VERIFIED
Whiteboard: [dont pass try][softblocker] → [dont pass try][softblocker][bugday0204]
The patch landed, just looks like the bug was never closed.  Thanks for catching that Ben and Aleksej!
Whiteboard: [dont pass try][softblocker][bugday0204] → [softblocker][bugday0204]
Whiteboard: [softblocker][bugday0204] → [softblocker][fx4-fixed-bugday]
blocking1.9.2: --- → .15+

Updated

8 years ago
blocking1.9.1: --- → .18+

Comment 15

8 years ago
Deferring to a future point release as we have run out of time. If this absolutely needs to be fixed and can land today or tomorrow, please send a note to release-drivers@mozilla.org
blocking1.9.1: .19+ → .20+
blocking1.9.2: .17+ → .18+
We still do need this one
blocking1.9.1: .20+ → .19+
blocking1.9.2: .18+ → .17+

Comment 18

8 years ago
This landing seems to kill JS "var ButtonImage = new Array()" function.

Comment 19

8 years ago
Sorry for my insufficient explanation.
After changes pushed with changeset a7a241b581ad, 'onmouseover' and 'onmouseout' function don't work.
Can you file a new bug with a testcase and CC me?
This turned the 1.9.2 reftest tinderboxen orange because of sync/async differences between trunk and branch. This patch should make them go green. It has r=bz and a=dveditz (both in person).
Attachment #525442 - Flags: review+
(Assignee)

Comment 23

8 years ago
Posted patch Fix for 1.9.1Splinter Review
This is a combined patch against 1.9.1 of all that was checked in for 1.9.2. I found out that nsCxPusher::PushNull doesn't exist on the 1.9.1 branch, and adding it isn't trivial, so I did the pushing and popping by hand instead. And some of the changed code didn't exist on 1.9.1 so there are a couple of hunks that are different here.
Attachment #526410 - Flags: review?(mrbkap)
(In reply to comment #20)
> Can you file a new bug with a testcase and CC me?

bug 649326 was filed on this, but was fixed by the patch in comment 21.
Comment on attachment 526410 [details] [diff] [review]
Fix for 1.9.1

Looks good. I was wondering if it might have been worth it to add a nsNullCxPusher class, but it only helps one case added in this patch, so probably not.
Attachment #526410 - Flags: review?(mrbkap) → review+
(Assignee)

Updated

8 years ago
Attachment #526410 - Flags: approval1.9.1.20?
Attachment #526410 - Flags: approval1.9.1.19?
Comment on attachment 526410 [details] [diff] [review]
Fix for 1.9.1

Approved for 1.9.1.19, a=dveditz for release-drivers

Please land ASAP so we can re-spin today.
Attachment #526410 - Flags: approval1.9.1.20?
Attachment #526410 - Flags: approval1.9.1.19?
Attachment #526410 - Flags: approval1.9.1.19+
I checked this with the attached testcase in build 1 (before the fix on 4/20) of 1.9.1.19. I do not see a crash. This is on Windows 7.

How does one cause the crash?
(It won't crash, but it should be enough to verify the fix here.)
Crash Signature: [@ nsImageLoadingContent::OnStartDecode]
Blocks: 691785
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.