1292869 - Crash in imgRequestProxy::GetImagePrincipal, repeatable CRASH at particular web page "directory.libsyn.com"

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P1)

Description

[benl]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0 SeaMonkey/2.47a1
Build ID: 20160731003204

Steps to reproduce:

Navigate to http://directory.libsyn.com/categories

Wait several seconds.  There will be on the page a spinny-thing that everyone knows means wait for something.




Actual results:

Seamonkey crashes.  Every time I've tried.


Expected results:

Seamonkey should show the webpage and whatever one is waiting for.

The page that crashes behaves like the main page, directory.libsyn.com , but that doesn't crash.

Crash reports (newest first):
bp-f63d4103-f08d-45db-9726-69a5f2160806
bp-4a24a1e6-3ef4-457c-9274-9662f2160806
bp-eb599c1b-8f38-4695-bfbc-cff152160806

Comment 1

[Rainer Bielefeld]

REPRODUCIBLE with official English SeaMonkey 2.47a1  (NT 6.1; Win64; WOW64; rv:50.0)  Gecko/20100101 Firefox/50.0 Build 20160729184603  (Default Classic Theme)  on German WIN7 64bit
REPRODUCIBLE with official en-US SeaMonkey 2.48a1  (NT 6.1; WOW64; rv:51.0)  Gecko/20100101 Firefox/51.0 Build 20160804000913  (Default Classic Theme)  on German WIN7 64bit (also with newly created User Profile in Safe Mode)

Crash reports (latest at the top):
<http://crash-stats.mozilla.com/report/index/bp-5fb733f1-0fdd-490d-b430-fc7742160806>
<http://crash-stats.mozilla.com/report/index/bp-3a8330fe-35ab-4740-a53c-9c5a52160806>
<http://crash-stats.mozilla.com/report/index/bp-fbfaeb70-135c-482a-8047-f7a202160806>

NOT reproducible with  en-US SeaMonkey 2.40 final Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) from official download page,  Gecko/20100101  Firefox/ 43.0  Build 20160120202951, (Default Classic Theme) on German WIN7 64bit

Comment 2

[Rainer Bielefeld]

Attachment: Screenshot Error Console

Already REPRODUCIBLE with official English SeaMonkey 2.47a1  (NT 6.1; Win64; WOW64; rv:50.0)  Gecko/20100101 Firefox/50.0 Build 20160729184603  (Default Classic Theme)  on German WIN7 64bit

a) The error shown in screenshot does not appear with 2.40, but I can't 
   tell whether the error is related to crash.
b) No DUPs found with <http://shortlinks.de/ed4h>
c) Also FF 51.0a1 (2016-08-04) affected, crashes because of 
   "EXCEPTION_ACCESS_VIOLATION_READ":
   bp-5de6bd3e-73d1-4093-a547-2f9ca2160806

Some more investigation required

Comment 3

[Rainer Bielefeld]

NEW for now, but DUP research here is beyond my skills

Comment 4

[Rainer Bielefeld]

d) currently only 1 crashing page known, so "major"
d) I am pretty sure that this is a new CORE bug, Version of appearance
   calculated from SM version

Comment 5

[Mayank Bansal]

Can repro in the latest Nightly

https://crash-stats.mozilla.com/report/index/e379cd60-4a11-4409-be88-a808e2160807

Comment 6

[Alice0775 White]

bp-0c0fae64-b781-4b6f-be09-d0f0b2160807

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=81d489064842a69044201690a18e315458698f53&tochange=0ac674ec465dcde9ec42bbed20ea6b22eb694296

Regressed by: Bug 471020

Comment 7

[Alice0775 White]

Crashes on Nightly51.0a1 and Aurora50.0a2 ubuntu14.04 64bit as well

bp-97a6d07f-c23d-4c2b-83c3-8b6c42160807
bp-58ac0d16-8434-416a-90c9-5570a2160807

Comment 8

[Carsten Book [:Tomcat]]

[Tracking Requested - why for this release]:

crashes also beta builds it seems

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Carsten Book [:Tomcat] from comment #8)
> [Tracking Requested - why for this release]:
> 
> crashes also beta builds it seems

Do you have some examples?  We can't see anything but 50 and 51?

Comment 10

[Christoph Kerschbaumer [:ckerschb]]

I can reproduce the crash, please see stacktrace underneath:

Program received signal SIGSEGV, Segmentation fault.
0x00007fffe6c7987b in imgRequestProxy::GetImagePrincipal (this=0x7fffbc0fa680, aPrincipal=0x7fffffffabd0) at /home/ckerschb/moz/mc/image/imgRequestProxy.cpp:671
671	  NS_ADDREF(*aPrincipal = GetOwner()->GetPrincipal());
(gdb) bt
#0  0x00007fffe6c7987b in imgRequestProxy::GetImagePrincipal (this=0x7fffbc0fa680, aPrincipal=0x7fffffffabd0) at /home/ckerschb/moz/mc/image/imgRequestProxy.cpp:671
#1  0x00007fffe7f49004 in mozilla::dom::CanvasRenderingContext2D::CachedSurfaceFromElement (this=0x7fffb23ed800, aElement=0x7fffd11c0260)
    at /home/ckerschb/moz/mc/dom/canvas/CanvasRenderingContext2D.cpp:4517
#2  0x00007fffe7f49d6a in mozilla::dom::CanvasRenderingContext2D::DrawImage (this=0x7fffb23ed800, aImage=..., aSx=0, aSy=0, aSw=0, aSh=0, aDx=0, aDy=0, 
    aDw=210,58333333333337, aDh=55,570601851851862, aOptional_argc=2 '\002', aError=...) at /home/ckerschb/moz/mc/dom/canvas/CanvasRenderingContext2D.cpp:4719
#3  0x00007fffe77b7771 in mozilla::dom::CanvasRenderingContext2D::DrawImage (this=0x7fffb23ed800, aImage=..., aDx=0, aDy=0, aDw=210,58333333333337, 
    aDh=55,570601851851862, aError=...) at /home/ckerschb/moz/mc-obj-ff-dbg/dist/include/mozilla/dom/CanvasRenderingContext2D.h:225
#4  0x00007fffe779ecf6 in mozilla::dom::CanvasRenderingContext2DBinding::drawImage (cx=0x7fffdbec3000, obj=..., self=0x7fffb23ed800, args=...)
    at /home/ckerschb/moz/mc-obj-ff-dbg/dom/bindings/CanvasRenderingContext2DBinding.cpp:4240
#5  0x00007fffe7ec1e31 in mozilla::dom::GenericBindingMethod (cx=0x7fffdbec3000, argc=5, vp=0x7fffd4068280)
    at /home/ckerschb/moz/mc/dom/bindings/BindingUtils.cpp:2812
#6  0x00007fffeb505566 in js::CallJSNative (cx=0x7fffdbec3000, native=0x7fffe7ec1c01 <mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*)>, 
    args=...) at /home/ckerschb/moz/mc/js/src/jscntxtinlines.h:235
#7  0x00007fffeb4ccc33 in js::InternalCallOrConstruct (cx=0x7fffdbec3000, args=..., construct=js::NO_CONSTRUCT)
    at /home/ckerschb/moz/mc/js/src/vm/Interpreter.cpp:453
#8  0x00007fffeb4ccf51 in InternalCall (cx=0x7fffdbec3000, args=...) at /home/ckerschb/moz/mc/js/src/vm/Interpreter.cpp:498
#9  0x00007fffeb4ccf7b in js::CallFromStack (cx=0x7fffdbec3000, args=...) at /home/ckerschb/moz/mc/js/src/vm/Interpreter.cpp:504
#10 0x00007fffeb4da0f9 in Interpret (cx=0x7fffdbec3000, state=...) at /home/ckerschb/moz/mc/js/src/vm/Interpreter.cpp:2873
#11 0x00007fffeb4cc8b1 in js::RunScript (cx=0x7fffdbec3000, state=...) at /home/ckerschb/moz/mc/js/src/vm/Interpreter.cpp:399
#12 0x00007fffeb4cccf3 in js::InternalCallOrConstruct (cx=0x7fffdbec3000, args=..., construct=js::NO_CONSTRUCT)
    at /home/ckerschb/moz/mc/js/src/vm/Interpreter.cpp:471
#13 0x00007fffeb4ccf51 in InternalCall (cx=0x7fffdbec3000, args=...) at /home/ckerschb/moz/mc/js/src/vm/Interpreter.cpp:498
#14 0x00007fffeb4ccfe2 in js::Call (cx=0x7fffdbec3000, fval=..., thisv=..., args=..., rval=...) at /home/ckerschb/moz/mc/js/src/vm/Interpreter.cpp:517
#15 0x00007fffeb2646a7 in JS::Call (cx=0x7fffdbec3000, thisv=..., fval=..., args=..., rval=...) at /home/ckerschb/moz/mc/js/src/jsapi.cpp:2852
#16 0x00007fffe7d2e962 in mozilla::dom::Function::Call (this=0x7fffc73a6400, cx=0x7fffdbec3000, aThisVal=..., arguments=..., aRetVal=..., aRv=...)
    at /home/ckerschb/moz/mc-obj-ff-dbg/dom/bindings/FunctionBinding.cpp:37
#17 0x00007fffe6de1811 in mozilla::dom::Function::Call<nsCOMPtr<nsISupports> > (this=0x7fffc73a6400, thisVal=..., arguments=..., aRetVal=..., aRv=..., 
    aExecutionReason=0x7fffebf7f214 "setTimeout handler", aExceptionHandling=mozilla::dom::CallbackObject::eReportExceptions, aCompartment=0x0)
    at /home/ckerschb/moz/mc-obj-ff-dbg/dist/include/mozilla/dom/FunctionBinding.h:70
#18 0x00007fffe6dc9c78 in nsGlobalWindow::RunTimeoutHandler (this=0x7fffcdc18000, aTimeout=0x7fffbf57cc80, aScx=0x7fffbeccdb00)
    at /home/ckerschb/moz/mc/dom/base/nsGlobalWindow.cpp:12305
#19 0x00007fffe6dca504 in nsGlobalWindow::RunTimeout (this=0x7fffcdc18000, aTimeout=0x7fffd41f4380) at /home/ckerschb/moz/mc/dom/base/nsGlobalWindow.cpp:12548
#20 0x00007fffe6dcaff6 in nsGlobalWindow::TimerCallback (aTimer=0x7fffbf5d7630, aClosure=0x7fffd41f4380) at /home/ckerschb/moz/mc/dom/base/nsGlobalWindow.cpp:12794
#21 0x00007fffe5437bd6 in nsTimerImpl::Fire (this=0x7fffbf5d7630) at /home/ckerschb/moz/mc/xpcom/threads/nsTimerImpl.cpp:521
#22 0x00007fffe54107c5 in nsTimerEvent::Run (this=0x7fffc727d170) at /home/ckerschb/moz/mc/xpcom/threads/TimerThread.cpp:286
#23 0x00007fffe5416705 in nsThread::ProcessNextEvent (this=0x7ffff6b7b500, aMayWait=false, aResult=0x7fffffffc6bf)
    at /home/ckerschb/moz/mc/xpcom/threads/nsThread.cpp:1058
#24 0x00007fffe547f939 in NS_ProcessNextEvent (aThread=0x7ffff6b7b500, aMayWait=false) at /home/ckerschb/moz/mc/xpcom/glue/nsThreadUtils.cpp:290
#25 0x00007fffe5bd3629 in mozilla::ipc::MessagePump::Run (this=0x7fffe1d93040, aDelegate=0x7ffff6b426e0) at /home/ckerschb/moz/mc/ipc/glue/MessagePump.cpp:96
#26 0x00007fffe5b43b2d in MessageLoop::RunInternal (this=0x7ffff6b426e0) at /home/ckerschb/moz/mc/ipc/chromium/src/base/message_loop.cc:232
#27 0x00007fffe5b43ac2 in MessageLoop::RunHandler (this=0x7ffff6b426e0) at /home/ckerschb/moz/mc/ipc/chromium/src/base/message_loop.cc:225
#28 0x00007fffe5b43a9b in MessageLoop::Run (this=0x7ffff6b426e0) at /home/ckerschb/moz/mc/ipc/chromium/src/base/message_loop.cc:205
#29 0x00007fffe8e9e170 in nsBaseAppShell::Run (this=0x7fffd3415320) at /home/ckerschb/moz/mc/widget/nsBaseAppShell.cpp:156
#30 0x00007fffe9e49e21 in nsAppStartup::Run (this=0x7fffd34102e0) at /home/ckerschb/moz/mc/toolkit/components/startup/nsAppStartup.cpp:284

Comment 11

[Christoph Kerschbaumer [:ckerschb]]

I'll take this one and have a closer look.

Comment 12

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1292869_imgrequestproxy_getimageprincipal.patch

Hey Jeff, 

the img request in question gets blocked by XCTO (see console message underneath) which then crashes  imgRequestProxy::GetImagePrincipal because the principal is a nullptr. I suppose we should use NS_IF_ADDREF. The callsite (CanvasRenderingContext2D.cpp:4517) actually checks for receiving a nullptr principal.

Visiting http://directory.libsyn.com/categories works now as expected.


%----snip----
The resource from “http://directory.libsyn.com/%3Cbr%20/%3E%3Cb%3EDeprecated%3C/b%3E:%20%20Non-static%20method%20Libsyn_Service_Asset::getCategoryImageUrl()%20should%20not%20be%20called%20statically,%20assuming%20$this%20from%20incompatible%20context%20in%20%3Cb%3E/www/libsyn/directory/application/views/scripts/categories/index.phtml%3C/b%3E%20on%20line%20%3Cb%3E37%3C/b%3E%3Cbr%20/%3Ehttps://ssl-static.libsyn.com/p/assets/platform/category/arts.png” was blocked due to MIME type mismatch (X-Content-Type-Options: nosniff).

Comment 13

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8779239 [details] [diff] [review]
bug_1292869_imgrequestproxy_getimageprincipal.patch

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

::: image/imgRequestProxy.cpp
@@ +667,5 @@
>    if (!GetOwner()) {
>      return NS_ERROR_FAILURE;
>    }
>  
> +  NS_IF_ADDREF(*aPrincipal = GetOwner()->GetPrincipal());

Let's do this instead:

nsCOMPtr<nsIPrincipal> principal = GetOwner()->GetPrincipal();
principal.forget(aPrincipal);

Comment 14

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1292869_imgrequestproxy_getimageprincipal.patch

Thanks; incorporated nit; carrying over r+ from jmuizelaar

Comment 15

[Carsten Book [:Tomcat]]

(In reply to Milan Sreckovic [:milan] from comment #9)
> (In reply to Carsten Book [:Tomcat] from comment #8)
> > [Tracking Requested - why for this release]:
> > 
> > crashes also beta builds it seems
> 
> Do you have some examples?  We can't see anything but 50 and 51?

sorry seems the automation was wrong here, did some tests and beta is fine

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/78c39a0de6a1
Fix null deref in imgRequestProxy::GetImagePrincipal. r=jmuizelaar

Comment 17

[[:philipp]]

adjusting the status flags according to comment #15

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/78c39a0de6a1

Comment 19

[Marcia Knous [:marcia]]

Tracking 50/51+ for this reproducible crash.

Comment 20

[Marcia Knous [:marcia]]

Sorry, the tracking flags didn't make the last submit.

Comment 21

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8779658 [details] [diff] [review]
bug_1292869_imgrequestproxy_getimageprincipal.patch

Approval Request Comment
Given that 50 is affected we should uplift that small fix to avoid crashing if XCTO: nosniff header blocks certain loads that also go through imgRequestProxy.

[Feature/regressing bug #]:
Bug 471020 - Add X-Content-Type-Options: nosniff support to Firefox

[User impact if declined]:
Users visiting http://directory.libsyn.com/categories or any other site where our implementation of XCTO nosniff blocks a load that also goes through imgRequstProxy.

[Describe test coverage new/current, TreeHerder]:
Manual verification that browser doesn't crash when visiting http://directory.libsyn.com/categories.

[Risks and why]:
Low, only fixing a potential null deref.

[String/UUID change made/needed]:
no

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8779658 [details] [diff] [review]
bug_1292869_imgrequestproxy_getimageprincipal.patch

This is already on 50, and I think 49 (in beta now) is unaffected (from looking at crash-stats and the regressing bug)

Comment 23

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8779658 [details] [diff] [review]
bug_1292869_imgrequestproxy_getimageprincipal.patch

Let's bring this fix to aurora though! Thanks philipp for pointing out it wasn't in 50 yet.

Comment 24

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e2249d2604d4

Comment 25

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #22)
> This is already on 50, and I think 49 (in beta now) is unaffected (from
> looking at crash-stats and the regressing bug)

Sorry, for the confusion. Obviously this was meant to be uplifted to 50 and not 49. Thanks for catching!