The default bug view has changed. See this FAQ.

Crash [@ nsEventStateManager::GetNextTabbableMapArea] with img usemap and tabindex

VERIFIED FIXED in mozilla1.9.2a1

Status

()

Core
Event Handling
--
critical
VERIFIED FIXED
9 years ago
6 years ago

People

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

Tracking

(5 keywords)

Trunk
mozilla1.9.2a1
x86
Windows XP
crash, fixed1.8.1.21, testcase, verified1.9.0.9, verified1.9.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse] null deref, crash signature)

Attachments

(5 attachments, 2 obsolete attachments)

(Assignee)

Description

9 years ago
Created attachment 334091 [details]
testcase

See testcase, which crashes in Firefox 2, Firefox 3 and current trunk build when tabbing in the document.

http://crash-stats.mozilla.com/report/index/adce7268-6b72-11dd-a999-001a4bd43ed6?p=1
0  	xul.dll  	nsEventStateManager::GetNextTabbableMapArea  	 content/events/src/nsEventStateManager.cpp:3955
1 	xul.dll 	nsEventStateManager::GetNextTabbableContent 	content/events/src/nsEventStateManager.cpp:3915
(Assignee)

Comment 1

9 years ago
Created attachment 334103 [details] [diff] [review]
patch
Attachment #334103 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 2

9 years ago
Created attachment 334104 [details] [diff] [review]
diff -w of patch

Or rather a hg diff -NprwU20 it seems.
perhaps early return?
if (!imageMap) {
  return nsnull;
}
(Assignee)

Comment 4

9 years ago
Created attachment 334106 [details] [diff] [review]
patch

I assume the img should not be tabbable, right? That's what IE seems to be doing.
Attachment #334103 - Attachment is obsolete: true
Attachment #334104 - Attachment is obsolete: true
Attachment #334106 - Flags: review?(Olli.Pettay)
Attachment #334103 - Flags: review?(Olli.Pettay)
(In reply to comment #4) 
> I assume the img should not be tabbable, right? That's what IE seems to be
> doing.
Does that have anything to do with this bug?

Attachment #334106 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Comment 6

9 years ago
Well, the patch makes the img with the non-existing usemap non-tabbable.
(Assignee)

Updated

9 years ago
Attachment #334106 - Flags: superreview?(roc)
how? It is just a null check for a case which would otherwise crash when
mapContent is used.
(Assignee)

Comment 8

9 years ago
You're right. Would probably better to file a new bug for that (different issue (if issue at all)).
(Assignee)

Comment 9

9 years ago
Hrm, with a slightly different testcase, I'm crashing in IE8 too.
Attachment #334106 - Flags: superreview?(roc) → superreview+
Flags: wanted1.9.0.x+
Flags: wanted1.8.1.x+
Flags: blocking1.9.0.2?
Flags: blocking1.8.1.17?
This appears to be an unexploitable null deref crash if this patch fixes it, and if that's true it's not a blocker. But we'd approve this simple fix if this works for the branches.
Flags: blocking1.9.0.2?
Flags: blocking1.8.1.17?
Whiteboard: [sg:nse] null deref
Assignee: nobody → martijn.martijn
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
(Assignee)

Updated

9 years ago
Flags: blocking1.9.1?
Not a blocker, but feel free to land this any time the tree is green.
Flags: blocking1.9.1? → blocking1.9.1-

Comment 12

8 years ago
Created attachment 351051 [details]
testcase 2 (crashes Firefox when loaded)

This testcase is fun in that it manages to trigger the tabbing code without any user interaction.
http://hg.mozilla.org/mozilla-central/rev/3a616f9b649e
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Attachment #334106 - Flags: approval1.9.1?
Comment on attachment 334106 [details] [diff] [review]
patch

a191=beltzner
Attachment #334106 - Flags: approval1.9.1? → approval1.9.1+
Keywords: checkin-needed
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f305bb515add

Does this same patch apply on 1.9.0 and 1.8.1?
Keywords: checkin-needed → fixed1.9.1
Not sure if this applies to 1.8.0.x as well, but I'll let those drivers determine whether they want it or not (since it's a null deref, maybe not even if it does apply).
Flags: wanted1.8.0.x?
Flags: blocking1.8.0.next?
Whiteboard: [sg:nse] null deref → [sg:nse] null deref [needs 1.9.0 and 1.8.1 patches]
Group: core-security
Flags: blocking1.9.0.7?
Flags: blocking1.9.0.7? → blocking1.9.0.7+
Martijn: Can you please work up 1.9.0 and 1.8.1 patches? Code freeze is technically tomorrow at 11:59pm...
Martijn: Please get a patch worked up for this in the next week or two so we can take it in 1.9.0.8.
Flags: blocking1.9.0.7+ → blocking1.9.0.8+
(Assignee)

Comment 19

8 years ago
Created attachment 360480 [details] [diff] [review]
1.9.0.x patch

Sorry, electricity went down yesterday, which made it impossible to submit a patch.
Do I need to ask review for these patches, I guess? Should I also add the tests?
I haven't tested this patch yet, I don't worry about the patch causing problems, though, more about the test.
Attachment #360480 - Flags: review?(dveditz)
(Assignee)

Comment 20

8 years ago
Created attachment 360481 [details] [diff] [review]
1.8.1.x patch

The 1.8.1 tree doesn't seem to have tests.
Attachment #360481 - Flags: review?(dveditz)
(Assignee)

Comment 21

8 years ago
Comment on attachment 360481 [details] [diff] [review]
1.8.1.x patch

Sorry, never mind, 1.8.1 is Firefox 2, which isn't supported anymore.
Attachment #360481 - Attachment is obsolete: true
Attachment #360481 - Flags: review?(dveditz)
Attachment #360481 - Attachment is obsolete: false
Attachment #360481 - Flags: review?(dveditz)
Comment on attachment 360481 [details] [diff] [review]
1.8.1.x patch

We still want this patch on 1.8.1 for Thunderbird 2 and for Firefox 2 which distros still ship.
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090209 Shiretoko/3.1b3pre
and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090209 Minefield/3.2a1pre
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
Comment on attachment 360480 [details] [diff] [review]
1.9.0.x patch

sr=dveditz
Attachment #360480 - Flags: review?(dveditz) → superreview+
Comment on attachment 360481 [details] [diff] [review]
1.8.1.x patch

sr=dveditz
Attachment #360481 - Flags: review?(dveditz) → superreview+
Attachment #360480 - Flags: approval1.9.0.8?
Attachment #360481 - Flags: approval1.8.1.next?
Attachment #360481 - Flags: approval1.8.0.next?
Comment on attachment 360480 [details] [diff] [review]
1.9.0.x patch

Approved for 1.9.0.8, a=dveditz for release-drivers
Attachment #360480 - Flags: approval1.9.0.8? → approval1.9.0.8+
Comment on attachment 360481 [details] [diff] [review]
1.8.1.x patch

Approved for 1.8.1.21, a=dveditz for release-drivers
Attachment #360481 - Flags: approval1.8.1.next? → approval1.8.1.next+
Whiteboard: [sg:nse] null deref [needs 1.9.0 and 1.8.1 patches] → [sg:nse] null deref
(Assignee)

Comment 28

8 years ago
Checking in content/events/src/nsEventStateManager.cpp;
/cvsroot/mozilla/content/events/src/nsEventStateManager.cpp,v  <--  nsEventStateManager.cpp
new revision: 1.743; previous revision: 1.742
done
Checking in content/events/test/Makefile.in;
/cvsroot/mozilla/content/events/test/Makefile.in,v  <--  Makefile.in
new revision: 1.19; previous revision: 1.18
done
RCS file: /cvsroot/mozilla/content/events/test/test_bug450876.html,v
done
Checking in content/events/test/test_bug450876.html;
/cvsroot/mozilla/content/events/test/test_bug450876.html,v  <--  test_bug450876.html
initial revision: 1.1
done

Checked into the 1.9.0.x tree.
Keywords: fixed1.9.0.8
(Assignee)

Comment 29

8 years ago
Checking in content/events/src/nsEventStateManager.cpp;
/cvsroot/mozilla/content/events/src/nsEventStateManager.cpp,v  <--  nsEventStateManager.cpp
new revision: 1.595.2.32; previous revision: 1.595.2.31
done

Checked into the 1.8.1.x tree.
Keywords: fixed1.8.1.21
Verified 1.9.0.8 using testcase attached and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.8pre) Gecko/2009031905 GranParadiso/3.0.8pre (.NET CLR 3.5.30729). Crashes 1.9.0.7 cleanly.
Keywords: fixed1.9.0.8 → verified1.9.0.8
Crash Signature: [@ nsEventStateManager::GetNextTabbableMapArea]
You need to log in before you can comment on or make changes to this bug.