Closed Bug 876489 Opened 11 years ago Closed 11 years ago

possible issue with current firefox and webrtc (SIP handling)

Categories

(Core :: WebRTC: Signaling, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla24
Tracking Status
firefox23 --- wontfix
firefox24 --- fixed
firefox-esr17 --- unaffected
b2g18 --- unaffected

People

(Reporter: posidron, Assigned: ehugg)

Details

(Keywords: sec-other, Whiteboard: [WebRTC][blocking-webrtc-][qa-][adv-main24-] upstream bug not used in Fx)

Attachments

(2 files)

Attached file mozilla.txt
We received this mail on security@mozilla.org


""""""

Hello,

I stumbled uppon a possible memory corruption problem in
mozilla-central, specifically in the call control handling of SIP
sessions. Since I do not understand the WebRTC API completely, I was not
able to exploit it, yet. Thus, there is a chance, that I have not seen
the place, which restricts the size of ccData.info to prevent a buffer
overflow in getDigits(). Nevertheless, adding a max_size parameter to
getDigits() might ensure, that the size check can be performed locally
in getDigits().

I attached a file, that summarizes the relevant code.

Kind regards,

 Martin

"""""



Are we using any of those switch-case events in processSessionEvent() where getDigits() might get called?
Flags: needinfo?(ethanhugg)
Keywords: sec-audit
Please reserve "sec-audit" for a bug report that represents a potentially risky code pattern that requires that we audit the rest of the codebase to make sure we don't have a problem. A report that represents an actual vulnerability should get a low-to-critical sec- rating. If a report might be a vulnerability but you don't know please don't add any sec- labels since that will bury the bug as "already triaged". If it's appropriately assigned things might work out fine anyway, but it could also result in a bug sitting unloved for some time.
Flags: sec-bounty?
Keywords: sec-audit
That function is one that is not used in Firefox (it supports SIP in the imported library, there are not code paths that would lead to it being called, as we do not support SIP).  As such, it would be an upstream bug only unless we decide in the future to enable SIP.  There are no current plans to do so, and doing so would proceeded by a bug-and-security analysis phase, as the SIP-specific code has not been tested against the changes made elsewhere to support WebRTC.

ehugg can verify
Flags: sec-bounty? → sec-bounty-
I double-checked with the debugger and Randell is correct that this getDigits call is never called in Firefox.

There has been no decision yet to revive the SIP stack in this code, neither has it been decided to remove it.  We do not pull from an upstream for this code so if we want it changed we should do it locally.  

I could fix this getDigits function by adding a max length parameter, but the test team would not be able to verify it.  If it pops up as an error in a code analysis tool then perhaps it would be worth fixing to avoid being reported again.

Randell, could you comment as to whether you would like a patch for this?
Flags: needinfo?(ethanhugg)
I think we've at least concluded at this point that this isn't a blocker.
Whiteboard: [WebRTC][blocking-webrtc-]
Comment on attachment 754859 [details] [diff] [review]
Signaling fix ccprovider::getDigits

Review of attachment 754859 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c
@@ +585,2 @@
>     char *endptr;
> +   unsigned len=0;

unsigned int, in keeping with the rest of the file (and for buffer_length as well)
Attachment #754859 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/dc659dc71d5d
Assignee: nobody → ethanhugg
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Whiteboard: [WebRTC][blocking-webrtc-] → [WebRTC][blocking-webrtc-][qa-]
Whiteboard: [WebRTC][blocking-webrtc-][qa-] → [WebRTC][blocking-webrtc-][qa-][adv-main24-]
Keywords: sec-other
Whiteboard: [WebRTC][blocking-webrtc-][qa-][adv-main24-] → [WebRTC][blocking-webrtc-][qa-][adv-main24-] upstream bug not used in Fx
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: