Closed Bug 1736220 Opened 3 years ago Closed 2 years ago

Crash in [@ sctp_timeout_handler]

Categories

(Core :: WebRTC, defect)

x86
All
defect

Tracking

()

RESOLVED FIXED
108 Branch
Tracking Status
firefox-esr102 109+ fixed
firefox108 --- fixed
firefox109 --- fixed

People

(Reporter: jesup, Assigned: bwc)

References

Details

(Keywords: crash, csectype-uaf, sec-high, Whiteboard: [post-critsmash-triage][adv-esr102.7-])

Crash Data

Attachments

(1 obsolete file)

UAF in the sctp_timeout() handler. Happening in current trunk.

Crash report: https://crash-stats.mozilla.org/report/index/145ecd01-c7c0-4eec-aa4a-682790211014

Reason: EXCEPTION_ACCESS_VIOLATION_WRITE

Top 7 frames of crashing thread:

0 xul.dll sctp_timeout_handler netwerk/sctp/src/netinet/sctputil.c:1811
1 xul.dll sctp_handle_tick netwerk/sctp/src/netinet/sctp_callout.c:172
2 xul.dll user_sctp_timer_iterate netwerk/sctp/src/netinet/sctp_callout.c:214
3 xul.dll sctp_create_thread_adapter netwerk/sctp/src/netinet/sctp_userspace.c:58
4 kernel32.dll BaseThreadInitThunk 
5 ntdll.dll __RtlUserThreadStart 
6 ntdll.dll _RtlUserThreadStart 

These kind of bugs have been fixed in usrsctp. We keep the refcount on the inp when the timer is running. So you might want to update...

And a couple of follow up fixes, where some fuzz testers found issues. So I'm not sure if cherry picking is worth the effort to find all relevant fixes.

Sounds like time for an update. Is there a good rev to use, or just tip?

Flags: needinfo?(tuexen)

What timeframe do you have?

There are two upcoming fixes you might be interested in:
(1) A cleanup of the locking.
(2) A fix of a bug showing up with PR-SCTP (see https://github.com/sctplab/usrsctp/issues/597)
After that, just using the tip should be fine.
I'll do (1) before (2).

Flags: needinfo?(tuexen)

Hey Michael, please let us know when you have a rev with those fixes.

Flags: needinfo?(tuexen)

(In reply to Jim Mathies [:jimm] from comment #6)

Hey Michael, please let us know when you have a rev with those fixes.

I'll do.

Flags: needinfo?(tuexen)
Assignee: nobody → jmathies
No longer blocks: webrtc-triage

This may be addressed by a libwebrtc update we're working on. TBD.

(In reply to Jim Mathies [:jimm] from comment #8)

This may be addressed by a libwebrtc update we're working on. TBD.

Google has been working on a new sctp library with a security focus. Our latest libwebrtc update pulls this in but it will require some integration work. Additionally it doesn't look like chrome is using it by default yet, so we'll wait for chrome use to shake that work out then schedule the integration work.

This is a UAF sec-high; it may well therefore be exploitable. We should take an update (even if it's a cherry-pick), instead of waiting more time for a possible replacement of the entire library -- which may well be many months, and google may well hit security issues with the new library which will slow our importing it.

Flags: needinfo?(jmathies)

(In reply to Randell Jesup [:jesup] (needinfo me) from comment #10)

This is a UAF sec-high; it may well therefore be exploitable. We should take an update (even if it's a cherry-pick), instead of waiting more time for a possible replacement of the entire library -- which may well be many months, and google may well hit security issues with the new library which will slow our importing it.

Nico, curious how much effort this would be? Looks like various fixes have landed upstream we might want.

Flags: needinfo?(jmathies) → needinfo?(na-g)

I'm working on an update over in bug 1795697 right now. So far, it seems to be going smoothly.

Flags: needinfo?(na-g)
Assignee: jmathies → nobody

Hey Byron, I'd like to assign this to you since you landed the sctp library update in Fx 108 (ref: bug 1795697); I think this is one of 2 bugs that we believe (fingers crossed) will be fixed when the library finishes rolling out Release. If you can keep an eye on this and verify that we don't see new crashes for a while so that we can close it, I'd really appreciate it. And if we do see crashes, please ping Michael Tüxen for debugging help. (I'm going to cross-post this message to the other bug.) Thanks!

Assignee: nobody → docfaraday

Hasn't shown up in 108 yet which is a good sign.

Still monitoring, no crashes yet in 108 Beta. Note we likely won't uplift the library update to 102 ESR, so this bug will stick around. We should be careful about how we handle this bug until the 102 release is deprecated in 2023-09-25.

There was only one crash in 107 beta and one in 106 beta so a lack of 108 beta crashes might just be luck-of-the-draw. We'll know after tomorrow's release.

It looks like the library update was just checked in to the ESR branch, for the 102.7 release in January.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Group: media-core-security → core-security-release
Target Milestone: --- → 108 Branch
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-esr102.7+r]
Attached file advisory.txt (obsolete) (deleted) —

Sorry for the unnecessary noise!

Updating this so that advisory scripts properly pull from 1795697

Whiteboard: [post-critsmash-triage][adv-esr102.7+r] → [post-critsmash-triage][adv-esr102.7-]
Attachment #9312403 - Attachment is obsolete: true
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: