Last Comment Bug 309574 - [FIX]Invalid read in nsJSEventListener::HandleEvent
: [FIX]Invalid read in nsJSEventListener::HandleEvent
Status: RESOLVED FIXED
: fixed1.8.0.4, fixed1.8.1
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: 1.8 Branch
: x86 Linux
: P1 normal (vote)
: mozilla1.8.1alpha1
Assigned To: Boris Zbarsky [:bz] (TPAC)
: Hixie (not reading bugmail)
Mentors:
Depends on:
Blocks:
  Show dependency tree / graph
 
Reported: 2005-09-21 21:54 PDT by Andrew Schultz
Modified: 2006-04-21 14:40 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Attachments
valgrind info (2.45 KB, text/plain)
2005-09-21 21:55 PDT, Andrew Schultz 		no flags Details
valgrind log for MOZILLA_1_8_BRANCH (2.46 KB, text/plain)
2006-04-11 07:13 PDT, Andrew Schultz 		no flags Details
Patch (1.26 KB, patch)
2006-04-14 18:49 PDT, Boris Zbarsky [:bz] (TPAC) 		jst: review+
jst: superreview+
jst: approval‑branch‑1.8.1+
dveditz: approval1.8.0.4+
Details | Diff | Splinter Review
View All Add an attachment (proposed patch, testcase, etc.)

Description Andrew Schultz 2005-09-21 21:54:16 PDT 
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.
Comment 1 Andrew Schultz 2005-09-21 21:55:13 PDT 
Created attachment 197000 [details]
valgrind info
Comment 2 Boris Zbarsky [:bz] (TPAC) 2006-04-10 14:46:53 PDT 
Andrew, do you still see this?  I can't reproduce...
Comment 3 Andrew Schultz 2006-04-10 19:22:32 PDT 
worksforme with trunk although I still see it with a 1.8 branch build.  Is there interest fixing this there?
Comment 4 Boris Zbarsky [:bz] (TPAC) 2006-04-10 20:17:26 PDT 
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.
Comment 5 Andrew Schultz 2006-04-10 22:25:29 PDT 
a build from 2005120204 does
a build from 2005120305 does not have the bug
Comment 6 Boris Zbarsky [:bz] (TPAC) 2006-04-10 22:35:58 PDT 
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?
Comment 7 Andrew Schultz 2006-04-11 07:13:00 PDT 
Created attachment 218027 [details]
valgrind log for MOZILLA_1_8_BRANCH

line numbers are only a bit off from before
Comment 8 Boris Zbarsky [:bz] (TPAC) 2006-04-11 09:47:28 PDT 
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...
Comment 9 Andrew Schultz 2006-04-14 18:00:59 PDT 
yes, the kungFuDeathGrip fixed the valgrind error
Comment 10 Boris Zbarsky [:bz] (TPAC) 2006-04-14 18:49:59 PDT 
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.
Comment 11 Johnny Stenback (:jst, jst@mozilla.com) 2006-04-17 14:03:07 PDT 
Comment on attachment 218492 [details] [diff] [review]
Patch

r+sr+a=jst
Comment 12 Boris Zbarsky [:bz] (TPAC) 2006-04-17 14:32:15 PDT 
Fixed on 1.8.1 branch.
Comment 13 Daniel Veditz [:dveditz] 2006-04-21 14:12:19 PDT 
Comment on attachment 218492 [details] [diff] [review]
Patch

approved for 1.8.0 branch, a=dveditz for drivers
Comment 14 Boris Zbarsky [:bz] (TPAC) 2006-04-21 14:40:34 PDT 
Fixed on 1.8.0 branch.
Note You need to log in before you can comment on or make changes to this bug.