Crash [@ nsTextControlFrame::SetValue] on branch with testcase2 from bug 373586

VERIFIED FIXED

Status

()

Core
Layout
--
critical
VERIFIED FIXED
10 years ago
6 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: smaug)

Tracking

(5 keywords)

1.8 Branch
x86
Windows XP
crash, testcase, topcrash, verified1.8.0.13, verified1.8.1.5
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.8.1.x +
wanted1.8.0.x +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:moderate?] requires chrome? uses deleted object, crash signature, URL)

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
See url, you need to download the testcase to your computer to get the crash, because of enhanced privileges.

This is a follow-up from bug 373586.

From comment 23 in that bug:
This crashes current branch builds when clicking on the button. You need to run
the testcase from chrome.
I get a [@ nsTextControlFrame::SetValue] (talkback ID: TB32917810X).
It doesn't crash on trunk, I think it's because of this patch.
I think this is currently nr. 29 of the hit list of crashes on branch:
http://talkback-public.mozilla.org/reports/firefox/FF2004/FF2004-topcrashers.html

Smaug, maybe this can be fixed by just a null-check?

Updated

10 years ago
Version: Trunk → 1.8 Branch
(Assignee)

Comment 1

10 years ago
I don't think so. I added a weakframe test and it showed that the frame got 
deleted. Probably something similar what bug 373586 did for ::SetValue is needed.
Assignee: nobody → Olli.Pettay
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
Component: Editor → Layout
(Assignee)

Comment 2

10 years ago
Created attachment 270554 [details] [diff] [review]
possible patch

Making the ::SetValue to have similar checks as what are currently on 
trunk. However with the patch using the testcase, the painting of the
page doesn't work correctly after pressing the button. (Painting
works ok when loading a new page.) The same painting problem happens
on trunk too, so I could just file a new (Non-SSensitive) bug for that.
The patch is needed anyway, though.
Attachment #270554 - Flags: review?(roc)
(Assignee)

Updated

10 years ago
Depends on: 386561
Attachment #270554 - Flags: superreview+
Attachment #270554 - Flags: review?(roc)
Attachment #270554 - Flags: review+
(Assignee)

Updated

10 years ago
Attachment #270554 - Flags: approval1.8.1.5?
Attachment #270554 - Flags: approval1.8.0.13?
waiting to give branch approval until this shakes out a bit on trunk and we figure out fixes for the painting regression.
Keywords: topcrash
Is there a way to trigger this situation without requiring chrome or UniversalXPConnect privileges? It's clearly using a deleted editor object, but if you need privileged code to get there then it's not an exploit.

Is the top-crash in this routine likely the same thing? If so then maybe there is a non-chrome way to exploit this. A lot of the talkbacks for this location die on the same "mEditor->SetFlags(savedFlags);" line I die on (and the rest say line "848" which isn't anywhere near this routine and which I'd chalk up to a glitch if it weren't so common and consistent a location). I guess the talkbacks pointing to the same object dereference makes me worry.

Whether we want to risk the regressions for the branch hinges on the answer to the above.

How bad is the painting regression? (shouldn't the bug have the regression keyword?).
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Whiteboard: [sg:moderate?] requires chrome? uses deleted object
Comment on attachment 270554 [details] [diff] [review]
possible patch

approved for 1.8.1.5 and 1.8.0.13, a=dveditz for release-drivers
Attachment #270554 - Flags: approval1.8.1.5?
Attachment #270554 - Flags: approval1.8.1.5+
Attachment #270554 - Flags: approval1.8.0.13?
Attachment #270554 - Flags: approval1.8.0.13+
(Assignee)

Updated

10 years ago
Keywords: fixed1.8.0.13, fixed1.8.1.5
(Assignee)

Updated

10 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Reporter)

Comment 6

10 years ago
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.5pre) Gecko/20070709
BonEcho/2.0.0.5pre

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.13pre) Gecko/20070710
Firefox/1.5.0.13pre

The testcase doesn't crash anymore after clicking on the button.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.0.13, fixed1.8.1.5 → verified1.8.0.13, verified1.8.1.5
Group: security
Flags: in-testsuite?

Updated

10 years ago
Duplicate of this bug: 383120
Crash Signature: [@ nsTextControlFrame::SetValue]
You need to log in before you can comment on or make changes to this bug.