Unsafe uses of mListener in nsWebSocketHandler.cpp

RESOLVED FIXED in Firefox 6

Status

()

Core
Networking: WebSockets
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jdm, Assigned: jdm)

Tracking

(Blocks: 1 bug)

unspecified
mozilla7
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox6 fixed)

Details

(crash signature)

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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);
(Assignee)

Updated

6 years ago
Blocks: 663769
Crash Signature: [@ mozilla::net::CallOnStop::Run]
(Assignee)

Updated

6 years ago
Summary: Unsafe uses of mListener in nsWebSocket.cpp → Unsafe uses of mListener in nsWebSocketHandler.cpp
(Assignee)

Comment 1

6 years ago
Created attachment 538965 [details] [diff] [review]
Avoid null dereferences in websocket handler.
(Assignee)

Updated

6 years ago
Attachment #538965 - Flags: review?(mcmanus)
(Assignee)

Updated

6 years ago
Blocks: 663918
Comment on attachment 538965 [details] [diff] [review]
Avoid null dereferences in websocket handler.

Review of attachment 538965 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #538965 - Flags: review?(mcmanus) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed

Updated

6 years ago
Assignee: nobody → josh
http://hg.mozilla.org/mozilla-central/rev/0715dd3bbfb8
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
The push broke the build, so I backed out all of its changesets.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Landed on inbound.
http://hg.mozilla.org/mozilla-central/rev/09428adfd4a8
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

6 years ago
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.
Attachment #538965 - Flags: approval-mozilla-aurora?
(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?
(Assignee)

Comment 9

6 years ago
We were seeing this in Fennec nightlies on mibbit.com.
Simple fix that fixes a mibbit crash in Fennec. Baked on trunk for 3 days

Comment 11

6 years ago
Comment on attachment 538965 [details] [diff] [review]
Avoid null dereferences in websocket handler.

Approved for mozilla-aurora
Attachment #538965 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
http://hg.mozilla.org/releases/mozilla-aurora/rev/3b8fa026b986
status-firefox6: --- → fixed
You need to log in before you can comment on or make changes to this bug.