Last Comment Bug 663893 - Unsafe uses of mListener in nsWebSocketHandler.cpp
: Unsafe uses of mListener in nsWebSocketHandler.cpp
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking: WebSockets (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla7
Assigned To: Josh Matthews [:jdm]
:
Mentors:
Depends on:
Blocks: 663918 663769
  Show dependency treegraph
 
Reported: 2011-06-13 10:49 PDT by Josh Matthews [:jdm]
Modified: 2011-06-21 14:19 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
Avoid null dereferences in websocket handler. (2.05 KB, patch)
2011-06-13 11:41 PDT, Josh Matthews [:jdm]
mcmanus: review+
christian: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Josh Matthews [:jdm] 2011-06-13 10:49:40 PDT
Both of the following are not sheltered in |if (mListener)| blocks, unlike other similar constructs:

868 nsCOMPtr<nsIRunnable> event =
869   new CallOnServerClose(mListener, mContext);
870 NS_DispatchToMainThread(event);

1317 nsCOMPtr<nsIRunnable> event =
1318   new CallOnStop(mListener, mContext, reason);
1319 NS_DispatchToMainThread(event);
Comment 1 Josh Matthews [:jdm] 2011-06-13 11:41:15 PDT
Created attachment 538965 [details] [diff] [review]
Avoid null dereferences in websocket handler.
Comment 2 Patrick McManus [:mcmanus] 2011-06-13 15:12:58 PDT
Comment on attachment 538965 [details] [diff] [review]
Avoid null dereferences in websocket handler.

Review of attachment 538965 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 3 Dão Gottwald [:dao] 2011-06-13 21:00:25 PDT
http://hg.mozilla.org/mozilla-central/rev/0715dd3bbfb8
Comment 4 :Ehsan Akhgari 2011-06-13 21:19:15 PDT
The push broke the build, so I backed out all of its changesets.
Comment 5 :Ehsan Akhgari 2011-06-13 21:25:07 PDT
Landed on inbound.
Comment 6 Dão Gottwald [:dao] 2011-06-13 21:29:02 PDT
http://hg.mozilla.org/mozilla-central/rev/09428adfd4a8
Comment 7 Josh Matthews [:jdm] 2011-06-16 11:05:59 PDT
Comment on attachment 538965 [details] [diff] [review]
Avoid null dereferences in websocket handler.

This fixes a potential crasher if websockets are misused by a page.
Comment 8 Patrick McManus [:mcmanus] 2011-06-16 11:12:28 PDT
(In reply to comment #7)
> Comment on attachment 538965 [details] [diff] [review] [review]
> Avoid null dereferences in websocket handler.
> 
> This fixes a potential crasher if websockets are misused by a page.

Can a page trigger this? I would think it would be limited to (broken) chrome code.. that's why I didn't a?
Comment 9 Josh Matthews [:jdm] 2011-06-16 11:13:27 PDT
We were seeing this in Fennec nightlies on mibbit.com.
Comment 10 Mark Finkle (:mfinkle) (use needinfo?) 2011-06-16 14:45:50 PDT
Simple fix that fixes a mibbit crash in Fennec. Baked on trunk for 3 days
Comment 11 christian 2011-06-16 15:12:37 PDT
Comment on attachment 538965 [details] [diff] [review]
Avoid null dereferences in websocket handler.

Approved for mozilla-aurora
Comment 12 Patrick McManus [:mcmanus] 2011-06-17 05:32:23 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/3b8fa026b986

Note You need to log in before you can comment on or make changes to this bug.