Godaddy webmail crash [EXCEPTION_ACCESS_VIOLATION_READ]

VERIFIED FIXED in Firefox 35

Status

()

defect
--
critical
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: toadyshadow101, Assigned: Ehsan)

Tracking

({crash, crashreportid})

34 Branch
mozilla38
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(firefox34 wontfix, firefox35+ verified, firefox36+ verified, firefox37+ verified, firefox38+ verified, relnote-firefox 35+)

Details

(crash signature)

Attachments

(3 attachments)

(Reporter)

Description

4 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0
Build ID: 20141126041045

Steps to reproduce:

Replied to email, Pasted small block of text from windows clipboard then
tried to delete a typo pressed backspace key 2 times.


Actual results:

Browser closed to desktop showing crash reporter.
https://crash-stats.mozilla.com/report/index/bp-4371a75f-4e2d-4fd5-bc1f-dd8ee2141217


Expected results:

Browser should not have crashed.

Unfortunately i have not been able to reproduce this issue other then the initial, However if i can i will post more information.

There seems to be others with the same signature, https://crash-stats.mozilla.com/report/list?product=Firefox&range_unit=days&range_value=28&signature=nsINode%3A%3ALength%28%29#tab-comments

Possibly caused by anti-virus

Comment 1

4 years ago
This stack looks like editor...
Severity: normal → critical
Crash Signature: [@ nsINode::Length()]
Component: Untriaged → Editor
Keywords: crash, crashreportid
Product: Firefox → Core

Comment 2

4 years ago
Ehsan - editor crashiness with identical stacks leading to nsINode::Length() (and implicitly, NodeType() - so looks like mNodeInfo is dead? Maybe I'm misreading the code...) any idea what's going on here? It looks like this started in 34, so that'd tie up nicely with bug 1055032... but I don't know how much change there is in editor-land and if that's a reasonable suspicion.

(confirming per multiple reports)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(ehsan)
Flags: needinfo?(Ms2ger)

Updated

4 years ago
Flags: needinfo?(Ms2ger)
(Reporter)

Comment 3

4 years ago
Was able to produce it on firefox 35.0b8 https://crash-stats.mozilla.com/report/index/bp-7611bc2f-4529-4f5e-9eec-432c62150110
I suspect that a debug build will trigger the MOZ_ASSERT(aParent) line. nsHTMLEditRules::JoinNodesSmart will need a null check.
And the regression was in mozilla-central changeset 4439b62ad770; before, JoinNodesSmart would call the variant of MoveNode that takes nsIDOMNode and checks for null, rather than the variant that takes nsINode and asserts.
(Assignee)

Comment 6

4 years ago
[Tracking Requested - why for this release]: Regression from bug 1055032 which landed in Firefox 34.  This can cause Firefox to crash.
Assignee: nobody → ehsan
Blocks: 1055032
Flags: needinfo?(ehsan)
Comment on attachment 8547679 [details] [diff] [review]
Null check the parent node in nsHTMLEditRules::JoinNodesSmart() before passing it to MoveNode

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

Test?
Attachment #8547679 - Flags: review?(roc) → review+
I believe the previous code might have early-returned, fwiw.
(Assignee)

Comment 10

4 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #8)
> Comment on attachment 8547679 [details] [diff] [review]
> Null check the parent node in nsHTMLEditRules::JoinNodesSmart() before
> passing it to MoveNode
> 
> Review of attachment 8547679 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Test?

I'm not sure how to reproduce the bug...

Toady, can you please give me some information on how to trigger the crash?  Specifically, what do you exactly copy to the clipboard in comment 0?  Thanks!
Flags: needinfo?(toadyshadow101)
(Assignee)

Comment 11

4 years ago
(In reply to :Ms2ger from comment #9)
> I believe the previous code might have early-returned, fwiw.

Yes, you're right <https://hg.mozilla.org/mozilla-central/rev/4439b62ad770#l1.18>.  I'll land the patch as an early return to keep the same behavior, although I'm not sure if it would make a difference either way.
(Assignee)

Comment 16

4 years ago
Ouch, sorry, and thanks for the fix!
Flags: needinfo?(ehsan)
https://hg.mozilla.org/mozilla-central/rev/1212b70ab9d9
https://hg.mozilla.org/mozilla-central/rev/7e143e1f0852
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
wontfix for 34 as 35 has already been released.

Ehsan - Please request uplift once you think this patch has been proven good on Nightly. Do you think this is severe enough to consider as a ride along for 35?
(Reporter)

Comment 19

4 years ago
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #10)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #8)
> > Comment on attachment 8547679 [details] [diff] [review]
> > Null check the parent node in nsHTMLEditRules::JoinNodesSmart() before
> > passing it to MoveNode
> > 
> > Review of attachment 8547679 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Test?
> 
> I'm not sure how to reproduce the bug...
> 
> Toady, can you please give me some information on how to trigger the crash? 
> Specifically, what do you exactly copy to the clipboard in comment 0? 
> Thanks!

So far i have only been able to trigger the crash 2 times one in 34.0.5 and one in 35.0 the exact steps to produce it are not exact and are inconsistent so not easily reproducible.

But the last crash in 35.0

I basically just typed text then backspaced it out, Then i would copy and paste a block of text into the form and backspace it out and repeat the above, Then out of the blue you press the backspace escaping 2 characters then the crash occurred.

The first crash i was composing an email and pasted in some text then went to backspace out a typo then Firefox closed and the crash reporter appeared, The second in 35.0 i basically have been trying everyday to narrow down the exact steps but sorry its far to inconsistent.

I have seen this message in the browser console "Security Error: Content at https://email19.asia.secureserver.net/pcompose.php may not load or link to chrome://starfieldzoom/content/logo.png." 

If there is a test build i can try i will see if i can reproduce it.
Flags: needinfo?(toadyshadow101)
(Assignee)

Comment 20

4 years ago
Thanks for the information, Toady.  The fix has been added to Nightly, which is an unofficial Firefox build built every night from the most recent code.  You can download it from <http://nightly.mozilla.org/>.  It would be great if you can test to see if you can get a crash running that build.  If you can, please include a link to the crash report on Nightly.  Thanks again!
Flags: needinfo?(ehsan)
(Assignee)

Comment 21

4 years ago
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #18)
> wontfix for 34 as 35 has already been released.
> 
> Ehsan - Please request uplift once you think this patch has been proven good
> on Nightly. Do you think this is severe enough to consider as a ride along
> for 35?

Given how safe the fix is, I think we should consider this as a ride-along for 35 if we do a dot release.  But I would like to wait for Toady to test Nightly given that we don't have an automated test for this...
(Assignee)

Updated

4 years ago
Flags: needinfo?(ehsan)
(Reporter)

Comment 22

4 years ago
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #21)
> (In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #18)
> > wontfix for 34 as 35 has already been released.
> > 
> > Ehsan - Please request uplift once you think this patch has been proven good
> > on Nightly. Do you think this is severe enough to consider as a ride along
> > for 35?
> 
> Given how safe the fix is, I think we should consider this as a ride-along
> for 35 if we do a dot release.  But I would like to wait for Toady to test
> Nightly given that we don't have an automated test for this...

I have tried for a total of 3hrs to reproduce it in nightly an are unable to produce the crash, So i would say you nailed it.

Vendor=Mozilla
Name=Firefox
RemotingName=firefox
CodeName=Nightly
Version=38.0a1
BuildID=20150115030228
SourceRepository=https://hg.mozilla.org/mozilla-central
SourceStamp=c1f6345f2803
(Assignee)

Comment 23

4 years ago
Great, thanks a lot for the bug report and for helping us verify the fix!
Flags: needinfo?(ehsan)
(Assignee)

Comment 24

4 years ago
Posted patch Branch patchSplinter Review
Approval Request Comment
[Feature/regressing bug #]: Bug 1055032.
[User impact if declined]: Crash when using software such as webmails, blog editing UI, wikipedia editing UI, etc.
[Describe test coverage new/current, TBPL]: We don't have automated tests for the fix since we don't know the exact content that triggers the bad behavior, but the crash is very well understood and the fix has been manually verified on trunk.
[Risks and why]: This is as low risk as it gets (it merely adds a null check for a pointer that will be dereferenced immediately after.)  I think it's safe to even take as a dot release ride along.
[String/UUID change made/needed]: None.
Attachment #8550290 - Flags: approval-mozilla-release?
Attachment #8550290 - Flags: approval-mozilla-beta?
Attachment #8550290 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 25

4 years ago
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #21)
> (In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #18)
> > wontfix for 34 as 35 has already been released.
> > 
> > Ehsan - Please request uplift once you think this patch has been proven good
> > on Nightly. Do you think this is severe enough to consider as a ride along
> > for 35?
> 
> Given how safe the fix is, I think we should consider this as a ride-along
> for 35 if we do a dot release.  But I would like to wait for Toady to test
> Nightly given that we don't have an automated test for this...

Needinfoing Lawrence since this may be time critical.
Flags: needinfo?(lmandel)
Transferring my ni to lsblakk, who is managing 35.
Flags: needinfo?(lmandel) → needinfo?(lsblakk)
Tracking and adding to the list of potential ride-along fixes if we need to do a chemspill or dot release.
Flags: needinfo?(lsblakk)
Attachment #8550290 - Flags: approval-mozilla-beta?
Attachment #8550290 - Flags: approval-mozilla-beta+
Attachment #8550290 - Flags: approval-mozilla-aurora?
Attachment #8550290 - Flags: approval-mozilla-aurora+
Comment on attachment 8550290 [details] [diff] [review]
Branch patch

We will probably do a 35.0.1. Taking it.
Attachment #8550290 - Flags: approval-mozilla-release? → approval-mozilla-release+
Toady, can you please try and see if this issue does reproduce for you in Firefox 35.0.1 build? I am unable to make a Godaddy webmail account.
Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/35.0.1-candidates/build1/win32/en-US/firefox-35.0.1.zip
Flags: needinfo?(toadyshadow101)
(Reporter)

Comment 34

4 years ago
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #33)
> Toady, can you please try and see if this issue does reproduce for you in
> Firefox 35.0.1 build? I am unable to make a Godaddy webmail account.
> Build:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/35.0.1-candidates/
> build1/win32/en-US/firefox-35.0.1.zip

Hello, Yes unable to produce the issue in 35.0.1 the issue appears to be resovled :)
Vendor=Mozilla
Name=Firefox
RemotingName=firefox
Version=35.0.1
BuildID=20150122214805
SourceRepository=https://hg.mozilla.org/releases/mozilla-release
SourceStamp=5ea9473a99ff
Flags: needinfo?(toadyshadow101) → needinfo?(bogdan.maris)
(In reply to Toady from comment #34)
> (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #33)
> > Toady, can you please try and see if this issue does reproduce for you in
> > Firefox 35.0.1 build? I am unable to make a Godaddy webmail account.
> > Build:
> > http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/35.0.1-candidates/
> > build1/win32/en-US/firefox-35.0.1.zip
> 
> Hello, Yes unable to produce the issue in 35.0.1 the issue appears to be
> resovled :)
> Vendor=Mozilla
> Name=Firefox
> RemotingName=firefox
> Version=35.0.1
> BuildID=20150122214805
> SourceRepository=https://hg.mozilla.org/releases/mozilla-release
> SourceStamp=5ea9473a99ff

Awsome, thanks Toady! Based on your verification from comment 22 and comment 34 I will mark this as verified. Would you be so kind to verify this on latest Aurora and beta as well? Just to finally close this bug.

Beta build: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/36.0b3-candidates/build1/win32/en-US/firefox-36.0b3.zip
Aurora build: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora/firefox-37.0a2.en-US.win32.zip
Status: RESOLVED → VERIFIED
Flags: needinfo?(bogdan.maris) → needinfo?(toadyshadow101)
(Reporter)

Comment 36

4 years ago
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #35)
> (In reply to Toady from comment #34)
> > (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #33)
> > > Toady, can you please try and see if this issue does reproduce for you in
> > > Firefox 35.0.1 build? I am unable to make a Godaddy webmail account.
> > > Build:
> > > http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/35.0.1-candidates/
> > > build1/win32/en-US/firefox-35.0.1.zip
> > 
> > Hello, Yes unable to produce the issue in 35.0.1 the issue appears to be
> > resovled :)
> > Vendor=Mozilla
> > Name=Firefox
> > RemotingName=firefox
> > Version=35.0.1
> > BuildID=20150122214805
> > SourceRepository=https://hg.mozilla.org/releases/mozilla-release
> > SourceStamp=5ea9473a99ff
> 
> Awsome, thanks Toady! Based on your verification from comment 22 and comment
> 34 I will mark this as verified. Would you be so kind to verify this on
> latest Aurora and beta as well? Just to finally close this bug.
> 
> Beta build:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/36.0b3-candidates/
> build1/win32/en-US/firefox-36.0b3.zip
> Aurora build:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora/
> firefox-37.0a2.en-US.win32.zip

Hello, Yes the issue is not present in Beta build & Aurora build you supplied.

The patches by Ehsan & Benjamin has resolved this issue. 

If you need me to run more tests please feel free to ask :)
Flags: needinfo?(toadyshadow101)
You need to log in before you can comment on or make changes to this bug.