895023 - crash in mozilla::image::VectorImage::GetIntrinsicRatio(nsSize*), with "display:none" on root node in SVG as an image

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Adnan Cehic]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/29.0.1547.22 Safari/537.36

Steps to reproduce:

Loaded a page that had multiple .svg files embedded through CSS. Trial and error pointed to a particular .svg as the culprit. Long story short, display="none" was present in the <svg> tag. Removing display="none" from <svg display="none"> fixed the crash issue.


Actual results:

Browser crashed.


Expected results:

Page should have loaded as it would in FF21, or any other browser.

Comment 1

[Robert Longson [:longsonr]]

Can you attach the testcase that causes it to crash please using the Add an Attachment button?

Comment 2

[Adnan Cehic]

Attachment: helper SVG file (to be used as an image)

SVG will load fine on it's own, but when it is present in an HTML document, browser crashes.

Comment 3

[Adnan Cehic]

Not entirely sure what a testcase is, so hopefully the responsible file and description are help enough.

Comment 4

[Robert Longson [:longsonr]]

Attachment: wrapper

Comment 5

[Robert Longson [:longsonr]]

You mean like this? It doesn't crash for me.

Comment 6

[Adnan Cehic]

Attachment: css code

Like so. I should have been more clear. My apologies :(

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: testcase (warning: crashes firefox)

I can reproduce.

Here's a testcase using <img>.

Comment 8

[Daniel Holbert [:dholbert]]

Crash report: bp-56d9ea20-2131-46c5-86cf-1cc0f2130717

It's a null-deref crash, so probably not exploitable, thankfully.

Comment 9

[Daniel Holbert [:dholbert]]

I can reproduce in Nightly as well as in the current release (Firefox 22). --> Setting "affected" flags on those branches & everything in between.

Comment 10

[Daniel Holbert [:dholbert]]

Looks like we're crashing because |rootFrame| is null.

Probably a regression from http://hg.mozilla.org/mozilla-central/rev/da9e03172d00 (bug 842850) which removed the null-check.

Comment 11

[Daniel Holbert [:dholbert]]

Attachment: fix v1

Here's a fix with two reftests-that-are-basically-crashtests -- one with <img> and one with background-image.

(I made them reftests that compare against "about:blank", just to be sure we don't paint random junk or the actual SVG content or anything.)

Comment 12

[Daniel Holbert [:dholbert]]

Attachment: fix v1a

(oops; I copypasted the wrong license block into the testcases. Fixed.)

Comment 13

[Daniel Holbert [:dholbert]]

https://tbpl.mozilla.org/?tree=Try&rev=56e7d6bc1aa6

Comment 14

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

Looking at this bug in Firefox 22 crashes Firefox, I assume from the attachments. So I can't open this bug except in Nightly.

Comment 15

[Daniel Holbert [:dholbert]]

Liz: I suspect you have BugzillaJS, configured to display image attachments inline in Bugzilla pages. In that configuration, viewing this bug page will crash (due to this very bug), in Firefox 22 through Nightly.  (Not sure why it didn't crash for you in nightly -- I'd expect it would have. Perhaps BugzillaJS got disabled when you switched to Nightly?)

Comment 16

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

Daniel: That is exactly it!   And I'm running Nightly with no add-ons.

Comment 17

[Lukas Blakk [:lsblakk] use ?needinfo]

Unless this is a high volume crash we're not going to track but will take a low risk uplift as high as we can depending on when it's ready.

Comment 18

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 777419 [details] [diff] [review]
fix v1a

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

Heh, interesting test case! The fix looks good.

Comment 19

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/mozilla-central/rev/e3c19a339b36

Comment 20

[Daniel Holbert [:dholbert]]

Comment on attachment 777419 [details] [diff] [review]
fix v1a

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 842850

User impact if declined: Crash (from null deref) when viewing certain SVG images.

Testing completed (on m-c, etc.): Local testing (& just landed on m-c, so this should be getting nightly testing starting tomorrow).

Risk to taking this patch (and alternatives if risky): Low. This just restores some null-checks that were accidentally removed as part of the regressing patch.

String or IDL/UUID changes made by this patch: none

Comment 21

[(Out Sep 17-23) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-aurora/rev/653b455a607a
https://hg.mozilla.org/releases/mozilla-beta/rev/d8eae64ca0e3

Comment 23

[u421692]

Verified fixed on:
Build: 
Firefox for Android 23.0b8_build2
Firefox for Android 24.0a2(2013-07-24)
Firefox for Android 25.0a1(2013-07-24)
Device: LG Nexus 4 (Android 4.2.2)/Samsung Galaxy TAB (Android 4.0.4)