1736220 - Crash in [@ sctp_timeout_handler]

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

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 

Comment 1

[Michael Tüxen]

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...

Comment 2

[Julien Cristau [:jcristau]]

Looks like https://github.com/sctplab/usrsctp/commit/939d48f9632d69bf170c7a84514b312b6b42257d?

Comment 3

[Michael Tüxen]

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.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 5

[Michael Tüxen]

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).

Comment 6

[Jim Mathies [:jimm]]

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

Comment 7

[Michael Tüxen]

(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.

Comment 8

[Jim Mathies [:jimm]]

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

Comment 9

[Jim Mathies [:jimm]]

(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.

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

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.

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

https://crash-stats.mozilla.org/report/index/d58fa75d-dd58-4c63-90be-aab6d0221006 may be the same crash, though I'm not sure

Comment 12

[Jim Mathies [:jimm]]

(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.

Comment 13

[Byron Campen [:bwc]]

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

Comment 14

[Maire Reavy [:mreavy]]

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!

Comment 15

[Jim Mathies [:jimm]]

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

Comment 16

[Jim Mathies [:jimm]]

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.

Comment 17

[Daniel Veditz [:dveditz]]

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.

Comment 18

[June Wilde (she/her) [:jewilde]]

Attachment: advisory.txt

Comment 19

[June Wilde (she/her) [:jewilde]]

Sorry for the unnecessary noise!

Updating this so that advisory scripts properly pull from 1795697