The default bug view has changed. See this FAQ.

[FIX]Invalid read in nsJSEventListener::HandleEvent

RESOLVED FIXED in mozilla1.8.1alpha1

Status

()

Core
DOM: Events
P1
normal
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: Andrew Schultz, Assigned: bz)

Tracking

({fixed1.8.0.4, fixed1.8.1})

1.8 Branch
mozilla1.8.1alpha1
x86
Linux
fixed1.8.0.4, fixed1.8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

12 years ago
Following part of the steps frmo bug 309564, valgrind reports an invalid read in
nsJSEventListener::HandleEvent (of freed memory).

Steps to reproduce:
2. Visit the linked URL ( http://demo.planzo.com )
3. Click a box on the calendar and type a comment then press <ENTER>
4. Double-click the event you entered in Step 3.

The page is heavily DHTML and also does AJAX.  I'll attach the info valgrind gives.
(Reporter)

Comment 1

12 years ago
Created attachment 197000 [details]
valgrind info
Andrew, do you still see this?  I can't reproduce...
(Reporter)

Comment 3

11 years ago
worksforme with trunk although I still see it with a 1.8 branch build.  Is there interest fixing this there?
Version: Trunk → 1.8 Branch
Probably... if we could figure out when this got fixed on trunk (e.g. could we test with nightlies?), that would be a great start.
(Reporter)

Comment 5

11 years ago
a build from 2005120204 does
a build from 2005120305 does not have the bug
Presumably when the JS event listener stuff got changed majorly in bug 241518...

Do we have any idea for the line number or anything like that?  I mean on the branch?
(Reporter)

Comment 7

11 years ago
Created attachment 218027 [details]
valgrind log for MOZILLA_1_8_BRANCH

line numbers are only a bit off from before
Does doing:

  nsCOMPtr<nsIJSEventListener> kungFuDeathGrip(this);

At the top of HandleEvent help?  It looks to me like we die in the event, then try to access our member...
(Reporter)

Comment 9

11 years ago
yes, the kungFuDeathGrip fixed the valgrind error
Created attachment 218492 [details] [diff] [review]
Patch

OK, I see why bug 241518 helped.  This patch should fix the valgrind warning too.

Drivers:  This is a memory read of deleted memory.  I _think_ it's not exploitable (it's just accessing a member variable).  But it might make sense to fix this on the 1.8.0 branch anyway.
Attachment #218492 - Flags: superreview?(jst)
Attachment #218492 - Flags: review?(jst)
Attachment #218492 - Flags: approval1.8.0.3?
Attachment #218492 - Flags: approval-branch-1.8.1?(jst)
Assignee: events → bzbarsky
Priority: -- → P1
Summary: Invalid read in nsJSEventListener::HandleEvent → [FIX]Invalid read in nsJSEventListener::HandleEvent
Target Milestone: --- → mozilla1.8.1alpha1
Comment on attachment 218492 [details] [diff] [review]
Patch

r+sr+a=jst
Attachment #218492 - Flags: superreview?(jst)
Attachment #218492 - Flags: superreview+
Attachment #218492 - Flags: review?(jst)
Attachment #218492 - Flags: review+
Attachment #218492 - Flags: approval-branch-1.8.1?(jst)
Attachment #218492 - Flags: approval-branch-1.8.1+
Fixed on 1.8.1 branch.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Comment on attachment 218492 [details] [diff] [review]
Patch

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #218492 - Flags: approval1.8.0.3? → approval1.8.0.3+
Fixed on 1.8.0 branch.
Keywords: fixed1.8.0.3
You need to log in before you can comment on or make changes to this bug.