848644 - (CVE-2013-0787) Use-after-free caused by us replacing the nsHTMLEditor's edit rules object while running scripts from the flush happening in nsEditor::IsPreformatted triggered by execCommand from web content

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Reed Loden [:reed]]

Placeholder for Pwn2Own bug found in Firefox by VUPEN at CanSecWest 2013.

Comment 1

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

Attachment: PoC (crash on load)

Comment 2

[Justin Dolske [:Dolske]]

document.designMode = "on";
...
document.execCommand("insertOrderedList",false,true);

Looks like a designMode / contentEditable / editor bug at first glance?

--> preemptive cc of ehsan & roc.

Some stuff with form <inputs>, but it very very vaguely looks like that's related to the heap spraying and not the actual flaw?

Comment 3

[Christian Holler (:decoder)]

Attachment: ASan Debug Trace with Symbols

Here's an ASan debug trace with symbols, taken from m-c rev 216ec69cc531.

Comment 4

[Christian Holler (:decoder)]

Memory freed in nsHTMLInputElement::SetValueInternal then used in nsHTMLEditRules::GetPromotedPoint.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Hmm.  So the PoC crashes on Firefox 18 for me but not in nightlies...

Comment 6

[Boris Zbarsky [:bzbarsky]]

OK, given comment 3 that last is just luck presumably.

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Does not crash: 20120101 Win32 nightly on Win7
Crashes: 20130101 Win32 nightly on Win7

Still narrowing this down...

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Does not crash: 20121005 Win32 nightly on Win7
Crashes: 20121006 Win32 nightly on Win7

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fd724f194a1f&tochange=2da1f2bde40e

(I *think* this is somewhat correct)

Comment 9

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Taking a guess from the regression range - it might be Core: Editor bug 796839.

Comment 10

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Christian Holler (:decoder) from comment #4)
> Memory freed in nsHTMLInputElement::SetValueInternal then used in
> nsHTMLEditRules::GetPromotedPoint.

In this changeset from bug 796839,

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

the file editor/libeditor/html/nsHTMLEditRules.cpp is changed.

And nsHTMLEditRules::GetPromotedPoint from comment 4 is found in editor/libeditor/html/nsHTMLEditRules.cpp

Comment 11

[(no longer active)]

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fd724f194a1f&tochange=2da1f2bde40e

Comment 12

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Christian Holler (:decoder) from comment #4)
> Memory freed in nsHTMLInputElement::SetValueInternal then used in
> nsHTMLEditRules::GetPromotedPoint.

> Taking a guess from the regression range - it might be Core: Editor bug
> 796839.

Or it could be bug 795610, from that regression window.

nsHTMLInputElement::SetValueInternal is defined in nsHTMLInputElement.cpp which was modified in bug 795610.

Comment 13

[u279076]

Setting Matt Wobensmith as QA contact for this bug. Please advise how we can help.

Comment 14

[(no longer active)]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #12)
> (In reply to Christian Holler (:decoder) from comment #4)
> > Memory freed in nsHTMLInputElement::SetValueInternal then used in
> > nsHTMLEditRules::GetPromotedPoint.
> 
> > Taking a guess from the regression range - it might be Core: Editor bug
> > 796839.
> 
> Or it could be bug 795610, from that regression window.
> 
> nsHTMLInputElement::SetValueInternal is defined in nsHTMLInputElement.cpp
> which was modified in bug 795610.

I don't think this bug is relevant...

Comment 15

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #13)
> Setting Matt Wobensmith as QA contact for this bug. Please advise how we can
> help.

Please confirm that:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012/10/2012-10-05-03-06-09-mozilla-central/firefox-18.0a1.en-US.win32.zip

does not crash but:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012/10/2012-10-06-03-05-34-mozilla-central/firefox-18.0a1.en-US.win32.zip

does crash, on Win7.

To get it to crash, try reloading, or closing the browser window and reloading the PoC again.

Comment 16

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I crashed today's Win32 binary on Win7 at:

bp-608c66ce-5f5a-4fbc-be88-ead642130307

Comment 18

[Al Billings [:abillings - ex-MoCo]]

This looks like ESR17 is unaffected based on the 10/05 date, which puts 17 on Aurora at that point.

Comment 19

[Daniel Veditz [:dveditz]]

Con(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #15)
> Please confirm that:
> 
> /nightly/2012/10/2012-10-05-03-06-09-mozilla-central/firefox-18.0a1.en-US.win32.zip
> 
> does not crash but:
> 
> /nightly/2012/10/2012-10-06-03-05-34-mozilla-central/firefox-18.0a1.en-US.win32.zip
> 
> does crash, on Win7.

On Win7 that is what I see. 10-06 crashes but -05 does not.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Attachment: Stack that shows what goes wrong

So the execCommand does some stuff that ends up calling IsPreformatted, which flushes style, which flushes pending resize events (presumably there's a pending resize from the "I blocked your popup" notification), which runs the resize handler, which runs page script while editor code is on the stack.  That page script calls document.open() and then document.write(), which reinitialized the editor and gives it a new editrules.  But the old editrules is still on the stack, and the only strong ref to it was from the editor, so it dies and now we're working with a deleted object and things are all bad.

If I hold a strong ref to the editrules on the stack in frame 49 of this callstack, I end up with a null-deref crash instead of a bogus-memory crash (because the mEditor of the editrules ends up null).

Comment 21

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Al Billings [:abillings] from comment #18)
> This looks like ESR17 is unaffected based on the 10/05 date, which puts 17
> on Aurora at that point.

Not true. I crashed ESR 17 nightly Win32 on Win7:

bp-719b5a3c-122e-4754-86cc-62daf2130307

Comment 22

[(no longer active)]

One problem here is that nsHTMLDocument::EditingStateChanged tries to reuse the nsHTMLEditor object if one is around previously, but we can't easily change that.  Putting a script blocker inside IsPreformatted is also not an option.

Comment 23

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

> (In reply to Al Billings [:abillings] from comment #18)
> > This looks like ESR17 is unaffected based on the 10/05 date, which puts 17
> > on Aurora at that point.
> 
> Not true. I crashed ESR 17 nightly Win32 on Win7:
> 
> bp-719b5a3c-122e-4754-86cc-62daf2130307

Bug 796839 was backported to Aurora. See bug 796839 comment 27.

Thus, everything from 17 onwards is likely affected assuming bug 796839 is indeed the regressor.

Comment 24

[(no longer active)]

Putting a script blocker inside ExecCommand might be an option though!

Comment 25

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Nominating for all tracking/status flags on all current branches in existence.

Comment 26

[Boris Zbarsky [:bzbarsky]]

> assuming bug 796839 is indeed the regressor.

Backing out part 4 of that bug locally seems to fix this particular testcase at first glance, so the assumption that it's the regressor is probably safe.

Comment 27

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Boris Zbarsky (:bz) from comment #26)
> > assuming bug 796839 is indeed the regressor.
> 
> Backing out part 4 of that bug locally seems to fix this particular testcase
> at first glance, so the assumption that it's the regressor is probably safe.

Adding a bunch of keywords, and setting Trunk as the latest branch that this bug affects.

Not setting this to block bug 796839 until this bug is public, but feel free to set it if it doesn't matter.

Comment 28

[Daniel Veditz [:dveditz]]

Given the speed of this release it's not going to matter.

Comment 29

[Boris Zbarsky [:bzbarsky]]

So a possible brute-force approach here is:

1)  Make sure to always hold on-stack strong refs to the edit rules when calling into it.  Most callers already do.  I've just audited the rest and will post a patch to fix them all.  That converts the crash from a "virtual call on deleted memory" to a "virtual call on null", which is a slight improvement.

2)  Add null-checks on mEditor in the edit rules as needed.

Comment 30

[Boris Zbarsky [:bzbarsky]]

Attachment: possible fix part 1. Always hold an on-stack strong reference to mRules when calling into it.

Comment 31

[(no longer active)]

Attachment: gzipped log of the test failures with a script blocker in ExecCommand

(In reply to :Ehsan Akhgari from comment #24)
> Putting a script blocker inside ExecCommand might be an option though!

Except that doing that breaks _tons_ of tests :(

Comment 32

[(no longer active)]

(In reply to Boris Zbarsky (:bz) from comment #29)
> So a possible brute-force approach here is:
> 
> 1)  Make sure to always hold on-stack strong refs to the edit rules when
> calling into it.  Most callers already do.  I've just audited the rest and
> will post a patch to fix them all.  That converts the crash from a "virtual
> call on deleted memory" to a "virtual call on null", which is a slight
> improvement.
> 
> 2)  Add null-checks on mEditor in the edit rules as needed.

ehsan
and the stupid fix would not be sufficient
ehsan
because the edit rules object is not the only thing that can get messed up
ehsan
if we hold a weak ref to a content node and the script causes that node to get deleted, then we lose
ehsan
and I'm sure we do a ton of those types of weak refs in the editor code :(

Comment 33

[(no longer active)]

Attachment: Do not flush in IsPreformatted

I think this is a better fix.

Comment 34

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 722053 [details] [diff] [review]
Do not flush in IsPreformatted

r=me

Comment 35

[(no longer active)]

Comment on attachment 722053 [details] [diff] [review]
Do not flush in IsPreformatted

I think this is the safest option that we currently have at hand.

Comment 36

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 722053 [details] [diff] [review]
Do not flush in IsPreformatted

That's r=me if it doesn't break anything, of course, as discussed.  ;)

Comment 37

[Daniel Veditz [:dveditz]]

Comment on attachment 722053 [details] [diff] [review]
Do not flush in IsPreformatted



a=dveditz

Comment 38

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cddf5f75843f

Comment 39

[Brendan Eich [:brendan]]

Can we statically analyze for functions and methods that must not flush, and annotate them to enforce the rule?

/be

Comment 40

[Alex Keybl [:akeybl]]

(In reply to :Ehsan Akhgari from comment #38)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/cddf5f75843f

Please advise QA on where the earliest available build to test this patch is (try build, m-i on-change, etc).

What areas should we test to ensure there are no regressions? This looks like a very simple fix, but I want to be sure we cover our bases.

Comment 41

[Boris Zbarsky [:bzbarsky]]

It's been suggested in the past, just never happened.  More precisely the suggestion was to have a way to tell whether a function can flush via static analysis...

Comment 42

[(no longer active)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/989f15f6ee61
https://hg.mozilla.org/releases/mozilla-beta/rev/1bc54ffd7a04
https://hg.mozilla.org/releases/mozilla-release/rev/0c7a6bfdfd8c
https://hg.mozilla.org/releases/mozilla-esr17/rev/fb56ba35db3b

Comment 43

[(no longer active)]

(In reply to Boris Zbarsky (:bz) from comment #41)
> It's been suggested in the past, just never happened.  More precisely the
> suggestion was to have a way to tell whether a function can flush via static
> analysis...

Perhaps this would be a(nother) reason for us to get serious about doing that! ;-)

Comment 44

[(no longer active)]

Ryan, I don't have b2g18* checkouts locally.  Would you mind doing the honors on those branches?  Thanks!

Comment 45

[Brendan Eich [:brendan]]

cc'ing bhacket in case sixgill can help.

/be

Comment 46

[(no longer active)]

(In reply to Alex Keybl [:akeybl] from comment #40)
> (In reply to :Ehsan Akhgari from comment #38)
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/cddf5f75843f
> 
> Please advise QA on where the earliest available build to test this patch is
> (try build, m-i on-change, etc).

This has landed on inbound <https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=cddf5f75843f>, aurora, beta, release and esr17.  I suggest focusing on testing this on the release branch.  I expect builds to be available starting from 1-2 hours from now, and between 3-4 hours from now on Windows.  Please watch <https://tbpl.mozilla.org/?tree=Mozilla-Release&rev=0c7a6bfdfd8c> and look for green B icons to know when the builds are done for each platform.

I strongly suggest testing this on Windows with the exploit test case to make sure that we've covered all cases.  The expected result on the exploit is no crash (and obviously, no exploit either -- not sure what the exploit shell code actually does; presumably it runs calc.exe or notepad.exe or something?)

> What areas should we test to ensure there are no regressions? This looks
> like a very simple fix, but I want to be sure we cover our bases.

Testing different richtext frameworks would be a good idea.  ckeditor, aloha, richtext mediawiki editor, etherpad, gmail, wordpress, etc.  That said, we do have a pretty good test coverage on the code path in question so I'm a bit less worried than I would be otherwise, but still we should look for regressions in richtext editors.

A regression would show up as an editing command accessed through either a keyboard shortcut (such as Ctrl+B, etc.) or an editing toolbar icon to not work, or stop working after editing a bit.  Please track the steps you're taking in testing carefully though, the editor is buggy as hell, so don't be surprised if you find bugs _not_ caused by this patch.  Recording the screen while testing is probably a good idea...

Comment 47

[(no longer active)]

To be perfectly clear, it _is_ possible that this patch breaks web content in ways that our automated and manual tests do not cover.  This patch is the best of all evil options that we have for the release channel, and I will be on the hook to address any possible regressions quickly.

Comment 48

[Alex Keybl [:akeybl]]

(In reply to :Ehsan Akhgari from comment #44)
> Ryan, I don't have b2g18* checkouts locally.  Would you mind doing the
> honors on those branches?  Thanks!

No urgency here - our partners are still picking up changes. But thanks for calling this out :)

Comment 49

[Alex Keybl [:akeybl]]

There's going to be a couple of things we do before spinning up a 19.0.2 candidate:

1) Wait for green across all platforms for at least one build
2) Test to make sure that a build with the patch resolves the exploitable crash
3) Decide how much regression testing is required (QA, pre-release audiences, etc.), based upon the risk of regression
- comment 46 directs QA testing very well, with that kind of targeted testing I think we can skip pre-release audience testing
4) Obviously land the patches to mozilla-release
- this is done in comment 42

In parallel, we should figure out how much further investigation is necessary to ensure there are no similarly exploitable paths (especially obvious ones), so that we can still take further change for a second candidate as necessary.
- Spoke with both bz/ehsan about this. They've done some code analysis tonight, but believe that Brendan's comment 39 would be the longer term investigation.

Comment 50

[(no longer active)]

(In reply to Alex Keybl [:akeybl] from comment #49)
> There's going to be a couple of things we do before spinning up a 19.0.2
> candidate:
> 
> 1) Wait for green across all platforms for at least one build
> 2) Test to make sure that a build with the patch resolves the exploitable
> crash
> 3) Decide how much regression testing is required (QA, pre-release
> audiences, etc.), based upon the risk of regression
> - comment 46 directs QA testing very well, with that kind of targeted
> testing I think we can skip pre-release audience testing
> 4) Obviously land the patches to mozilla-release
> - this is done in comment 42

Yeah, FWIW I don't expect that the pre-release audience can find any regressions that our targeted QA cannot, so it makes sense to me not to block on that.

> In parallel, we should figure out how much further investigation is
> necessary to ensure there are no similarly exploitable paths (especially
> obvious ones), so that we can still take further change for a second
> candidate as necessary.
> - Spoke with both bz/ehsan about this. They've done some code analysis
> tonight, but believe that Brendan's comment 39 would be the longer term
> investigation.

Amen!

Comment 51

[(no longer active)]

Boris, can you please file another bug and attach your patch there?  I would still like to take that patch but I need to review it carefully...

Comment 52

[Daniel Veditz [:dveditz]]

(In reply to :Ehsan Akhgari from comment #46)
> not sure what the exploit shell code actually does; presumably
> it runs calc.exe or notepad.exe or something?)

ZDI did not give us a working exploit, this PoC just crashes.

Comment 53

[(no longer active)]

I forgot to mention here that I ran all of the editor tests locally and they all passed with this patch.

Comment 54

[Jesse Ruderman]

(In reply to Brendan Eich [:brendan] from comment #39)
> Can we statically analyze for functions and methods that must not flush, and
> annotate them to enforce the rule?

Something like bug 477432 or bug 542364?

If a static analysis is hard, I'd rather have assertions than nothing.

Comment 55

[Alex Keybl [:akeybl]]

We're going to end up spinning a 19.0.2 for both desktop and mobile, but Lukas and I weren't able to repro with the PoC on Android.

Is there any reason to believe this Fennec will be any less impacted? Our decision to actually release the mobile build candidate will depend on that answer.

Comment 56

[Lukas Blakk [:lsblakk] use ?needinfo]

Tested this on https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-release-macosx64/ and was not able to reproduce the crash so will go ahead with kicking off the 19.0.2 builds from the changset with Ehsan's patch applied.  Fwiw, I tested on the current Aurora* and was not able to crash there either and this PoC was not showing the same thing as it does in Firefox 19.0.1 (or the patched tinderbox build) and it also does not crash - not sure what this means, but thought it worth mentioning.

* Built from http://hg.mozilla.org/releases/mozilla-aurora/rev/357d21b20bf0

Comment 57

[Aaron Train [:aaronmt]]

(In reply to Alex Keybl [:akeybl] from comment #55)
> We're going to end up spinning a 19.0.2 for both desktop and mobile, but
> Lukas and I weren't able to repro with the PoC on Android.
> 

Confirming neither here; builds tested release → central (ARMv7/6)

Comment 58

[(no longer active)]

I forgot to mention that one trick to reproduce this with the attached PoC is to have a single tab open when running the test.  Basically, the popup block infobar that changes the size of the viewport as the test is running is crucial to be able to reproduce this.

Comment 59

[Christian Holler (:decoder)]

Comment on attachment 722053 [details] [diff] [review]
Do not flush in IsPreformatted

Verified the fix on Linux using two ASan builds. The version with the patch shows no use-after-free occurring anymore.

Comment 60

[Boris Zbarsky [:bzbarsky]]

> Is there any reason to believe this Fennec will be any less impacted? 

For the underlying bug, no.

As for PoC testcase, it depends on window.open() triggering a resize of the content area of the web page it's called from.  This is true in desktop firefox but I'd be it's not on Android.  So a different vector would be needed to get the flush to trigger the script execution under editor code there.

Comment 61

[Alex Keybl [:akeybl]]

(In reply to :Ehsan Akhgari from comment #42)
> https://hg.mozilla.org/releases/mozilla-esr17/rev/fb56ba35db3b

Just realized - this needs to land to GECKO1703_2013021512_RELBRANCH as well, so that 17.0.4 doesn't pick up any of the Firefox 20 security fixes.

Comment 63

[Boris Zbarsky [:bzbarsky]]

> Just realized - this needs to land to GECKO1703_2013021512_RELBRANCH as well

https://hg.mozilla.org/releases/mozilla-esr17/rev/19c96497b96a

Comment 64

[Ryan VanderMeulen [:RyanVM]]

(In reply to :Ehsan Akhgari from comment #44)
> Ryan, I don't have b2g18* checkouts locally.  Would you mind doing the
> honors on those branches?  Thanks!

Not a problem, of course. I'll get it with my other uplifts today. :)

Comment 65

[(no longer active)]

(In reply to Jesse Ruderman from comment #54)
> If a static analysis is hard, I'd rather have assertions than nothing.

Assertions will not be a very strong defense, since they require us to hit bad code paths like this to trigger them.

Comment 66

[Boris Zbarsky [:bzbarsky]]

And for Thunderbird, the GECKO1703_2013021513_RELBRANCH branch:

https://hg.mozilla.org/releases/mozilla-esr17/rev/2c3766cfba66

Comment 67

[Boris Zbarsky [:bzbarsky]]

Attachment: Subframe for PoC that won't depend on the browser UI

Comment 68

[Boris Zbarsky [:bzbarsky]]

Attachment: PoC that doesn't depend on browser UI

This crashes my Firefox on Android.

Comment 70

[Mihaela Velimiroviciu (:mihaelav)]

Mozilla/5.0 (X11; Linux i686; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130307023931

Firefox 19.0.2/Ubuntu 12.10 doesn't crash when loading PoC test file.

Comment 72

[Virgil Dicu [:virgil] [QA]]

Verified the crash for Firefox 19.0 (20130215130331) and 19.0.1 (20130226172142) by using the attachment from comment 1.

No crashes for Firefox 19.0.2 (20130307023931) after loading PoC test file.

Verified on:
Mac OS X 10.8 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:19.0) Gecko/20100101 Firefox/19.0
Windows 7 x64 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0
Windows 8 x86 Mozilla/5.0 (Windows NT 6.2; rv:19.0) Gecko/20100101 Firefox/19.0

Comment 73

[(no longer active)]

I filed bug 848788 for a mechanism to only block script execution.  If we had such an infrastructure, fixing this and similar bugs would be a lot easier and safer.

Comment 74

[Aaron Train [:aaronmt]]

Aye, took a bit but hit the crash (via attachment #722231 [details]) on my LG Nexus 4 (Android 4.2) (trunk 03.07)

bp-d1862f75-46c8-4425-ba6f-40dd62130307

https://crash-stats.mozilla.com/report/index/bp-d1862f75-46c8-4425-ba6f-40dd62130307

Comment 75

[u279076]

Based on comments in this bug and the results of testing seen here[1] I'm calling this verified fixed for Firefox 19. Please advise if there is something more we need to be checking before signing off Firefox 19.0.2 for release.

1. https://etherpad.mozilla.org/19-0-2

Comment 76

[Aaron Train [:aaronmt]]

Patch attachment #722053 [details] [diff] [review] in mozilla-release 19.0.2 works for me on Android (ARMv7/ARMv6); likewise verified fixed on firefox19.

Comment 77

[Ryan VanderMeulen [:RyanVM]]

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

Comment 78

[u279076]

Confirmed reproducible with Firefox 20.0b3 on Linux.
Verified fixed with Firefox 20.0b4#1 on Linux.

Comment 79

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/a0b06192f882
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/b2a7c1e404b7

Comment 80

[(no longer active)]

Why was status-b2g18-v1.0.0 marked as wontfix?

Comment 81

[Andrew McCreight [:mccr8]]

(In reply to :Ehsan Akhgari from comment #80)
> Why was status-b2g18-v1.0.0 marked as wontfix?

IIUC, that branch isn't being used any more.

Comment 82

[Alex Keybl [:akeybl]]

(In reply to Andrew McCreight [:mccr8] from comment #81)
> (In reply to :Ehsan Akhgari from comment #80)
> > Why was status-b2g18-v1.0.0 marked as wontfix?
> 
> IIUC, that branch isn't being used any more.

That's correct.

Comment 83

[Alex Keybl [:akeybl]]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #75)
> Based on comments in this bug and the results of testing seen here[1] I'm
> calling this verified fixed for Firefox 19. Please advise if there is
> something more we need to be checking before signing off Firefox 19.0.2 for
> release.
> 
> 1. https://etherpad.mozilla.org/19-0-2

Spoke with Anthony about this on IRC. Given the verification testing at this etherpad, we can consider this bug verified and continue testing for regressions as we find the time.

Comment 84

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 722039 [details] [diff]
possible fix part 1.  Always hold an on-stack strong reference to mRules when calling into it.

> Boris, can you please file another bug and attach your patch there? 

Bug 848895.

Comment 85

[u279076]

Confirmed reproducible with Firefox Aurora 21.0a2 2013-03-06.
Verified fixed with Firefox Aurora 21.0a2 2013-03-07.

Comment 86

[u279076]

I can still reproduce this crash with Firefox Nightly 22.0a2 2013-03-07. Did this land in time for today's Nightlies? Is there another 22.0a2 build I can use for verification?

Comment 87

[Boris Zbarsky [:bzbarsky]]

> Did this land in time for today's Nightlies? 

No.

> Is there another 22.0a2 build I can use for verification?

The builds at http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1362676792/ should have this patch, for Windows.

Comment 88

[(no longer active)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #86)
> I can still reproduce this crash with Firefox Nightly 22.0a2 2013-03-07. Did
> this land in time for today's Nightlies? Is there another 22.0a2 build I can
> use for verification?

The patch was merged to central just this morning, so that is expected.  You should not be able to reproduce on recent central/inbound tinderbox builds.  This will be in tonight's Nightly.

Comment 89

[juan becerra [:juanb]]

Verified for 20.b4 candidates. Tried earlier betas, which crashed with the testcase. These didn't:
Mozilla/5.0 (Windows NT 6.1; WOW64; 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.7; rv:20.0) Gecko/20100101 Firefox/20.0

Comment 90

[u279076]

Thanks for the pointer Boris and Ehsan. I've verified fixed for Firefox Nightly 22.0a2 using the following builds:
> ftp://ftp.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-linux64/1362676792/
> ftp://ftp.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-win32/1362676792/
> ftp://ftp.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-macosx64/1362676792/

Comment 91

[Aaron Train [:aaronmt]]

Confirming reproducible with Firefox Beta for Android (mozilla-beta) 20.0 Beta 3 (ARMv7/ARMv6)
Verified Fixed with Firefox Beta for Android (mozilla-beta) 20.0 Beta 4 (ARMv7/ARMv6)

Comment 92

[Matt Wobensmith [:mwobensmith][:matt:]]

FF17 ESR verified fixed using today's nightly build on the following platforms:

MacOS 10.8
Win7 32
Win8 64
Ubuntu 12.04 64 

Attachment no longer crashes, and a spot check of ~20 top URLs was performed per platform to verify no broken functionality.

Comment 93

[(no longer active)]

Attachment: Bug 848644 - Add a crashtest based on the test case for the bug

Comment 94

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6cf11160e686
Add a crashtest based on the test case for the bug

Comment 95

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/6cf11160e686