Closed Bug 1026774 Opened 5 years ago Closed 5 years ago

malloc of undefined size in stun_get_mib_addrs in rare cases

Categories

(Core :: WebRTC: Networking, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
firefox34 --- wontfix
firefox35 --- fixed
firefox-esr31 --- wontfix
b2g-v1.4 --- wontfix
b2g-v2.0 --- wontfix
b2g-v2.0M --- wontfix
b2g-v2.1 --- fixed
b2g-v2.1S --- fixed
b2g-v2.2 --- fixed

People

(Reporter: bwc, Assigned: bwc)

Details

(Keywords: csectype-uninitialized, sec-moderate, Whiteboard: [adv-main35+][b2g-adv-main2.2+])

Attachments

(1 file)

If this call fails, |needed| is left uninitialized, and then passed to malloc (errx only prints to stderr, it does not bail).

http://dxr.mozilla.org/mozilla-central/source/media/mtransport/third_party/nICEr/src/stun/addrs.c#242

Granted, this failure should be rare, but I am not totally sure how rare.
So, if the malloc call doesn't crash, I would expect us to deref null here:

http://dxr.mozilla.org/mozilla-central/source/media/mtransport/third_party/nICEr/src/stun/addrs.c#254

This is probably just a straightforward crash bug.
So, at absolute most this is a DoS (if they can force you to allocate a ton of memory, and there are far easier ways to do that) or a null deref (which isn't a sec issue).
Well, if we allocate a ton of memory, we'll then start trying to parse it, which could send us into the weeds. If we get past here by sheer luck:

http://dxr.mozilla.org/mozilla-central/source/media/mtransport/third_party/nICEr/src/stun/addrs.c#254

We might end up going off into the weeds using random data.
Assignee: nobody → docfaraday
Status: NEW → ASSIGNED
Comment on attachment 8481586 [details] [diff] [review]
Return errors on some rare failure cases in stun_get_mib_addrs.

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

https://tbpl.mozilla.org/?tree=Try&rev=275586fa59fc
Attachment #8481586 - Flags: review?(martin.thomson)
Attachment #8481586 - Flags: review?(martin.thomson) → review+
https://hg.mozilla.org/mozilla-central/rev/d9e48e922e0d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Did this impact ESR31?
Flags: needinfo?(docfaraday)
Yes.
Flags: needinfo?(docfaraday)
Whiteboard: [adv-main35+]
Would this be worth backporting to b2g34 for v2.1 still?
I suspect the existing patch will apply cleanly there, so why not?
Flags: needinfo?(docfaraday)
Comment on attachment 8481586 [details] [diff] [review]
Return errors on some rare failure cases in stun_get_mib_addrs.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

   Initial webrtc landing

User impact if declined: 

   Potential use of uninitialized memory causing crashes or other undefined behavior in rare cases.

Testing completed: 

   The usual CI testing.

Risk to taking this patch (and alternatives if risky): 

   Basically no risk.

String or UUID changes made by this patch:

   None.
Attachment #8481586 - Flags: approval-mozilla-b2g34?
Attachment #8481586 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
Whiteboard: [adv-main35+] → [adv-main35+][b2g-adv-main2.2+]
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.