[Home]

Summary:ASTERISK-28495: res_pjsip_t38: 200 OK with SDP answer with declined stream causes crash
Reporter:Alexei Gradinari (alexei gradinari)Labels:patch security
Date Opened:2019-08-05 04:52:26Date Closed:2019-09-05 07:52:15
Priority:BlockerRegression?Yes
Status:Closed/CompleteComponents:Resources/res_pjsip_t38
Versions:16.5.0 Frequency of
Occurrence
Related
Issues:
is related toASTERISK-28506 asterisk crashes at random frequent intervals
is related toASTERISK-28209 res_pjsip_t38: Passthrough causes crash when re-invite collision
is related toASTERISK-28473 res_pjsip_t38: Crash on Asterisk 16.4
is related toASTERISK-28517 Asterisk segfault in t38_interpret_parameters at res_pjsip_t38.c:457
Environment:Attachments:( 0) ast-2019-004.patch
( 1) AST-2019-004.pdf
( 2) gdb.txt.xz
( 3) t38.diff
( 4) testsuite.tar.xz
Description:The asterisk doesn't check if there is media session with type 'image'
on receiving 200 reply on T.38 ReInvite.
If SDP contains 'm=image 0 udptl t38' the asterisk crashes.

My patch fixes only one place of code where t38_change_state is called without checking session_media variable.
I think t38_change_state should check session_media parameter before use it.
And I think need to check other places where active_media_state->default_session[AST_MEDIA_TYPE_IMAGE] is used.
Comments:By: Asterisk Team (asteriskteam) 2019-08-05 04:52:30.957-0500

This issue has been automatically restricted and set to a blocker due to being a security type issue. If this is not a security vulnerability issue it will be moved to the appropriate issue type when triaged.

By: Alexei Gradinari (alexei gradinari) 2019-08-05 10:37:30.943-0500

This patch fixes one place of code where t38_change_state is called
without checking session_media variable.

By: Kevin Harwell (kharwell) 2019-08-20 15:35:11.035-0500

[~alexei gradinari] I've attached a patch ([^t38.diff]) that should hopefully take care of all the cases if you want to look it over/test it out.

Also please review what will be the announcement document ([^AST-2019-004.pdf]) and let me know if you'd like me to add/remote/update anything on it.

Thanks!

By: Alexei Gradinari (alexei gradinari) 2019-08-21 09:05:16.712-0500

About t38_reinvite_response_cb.
If we just return on !session_media could the media_state be leaked?
Shouldn't we do the same as on T.38 reject?

/* If no session_media then response contained a declined stream, so disable */
t38_change_state(session, session_media, state, session_media ? T38_REJECTED : T38_DISABLED);

/* Abort this attempt at switching to T.38 by resetting the pending state and freeing our stored away active state */
ast_sip_session_media_state_free(state->media_state);
state->media_state = NULL;
ast_sip_session_media_state_reset(session->pending_media_state);


By: Alexei Gradinari (alexei gradinari) 2019-08-21 09:18:51.090-0500

I think
1. also need to check session_media in t38_change_state
2. as session_media is used only for T38_PEER_REINVITE and T38_ENABLED, not need to pass session_media to t38_change_state on other states
for example
t38_change_state(data->session, NULL, state, T38_REJECTED);
t38_change_state(data->session, NULL, state, T38_DISABLED);

By: Kevin Harwell (kharwell) 2019-08-21 10:20:02.128-0500

Thanks for the feedback!

For #1 I don't think we need to check again for NULL in the t38_change_state function as we are checking it prior to entering it, so all cases should now be accounted for.

Also you have been added to the appropriate group on gerrit so should now have access to the review itself beneath the Security-Asterisk repo, if you want to move the discussion there. You might need to logout/log back in to see it.

By: Friendly Automation (friendly-automation) 2019-09-05 07:52:16.247-0500

Change 12841 merged by George Joseph:
AST-2019-004 - res_pjsip_t38.c: Add NULL checks before using session media

[https://gerrit.asterisk.org/c/asterisk/+/12841|https://gerrit.asterisk.org/c/asterisk/+/12841]

By: Friendly Automation (friendly-automation) 2019-09-05 07:52:21.990-0500

Change 12842 merged by George Joseph:
AST-2019-004 - res_pjsip_t38.c: Add NULL checks before using session media

[https://gerrit.asterisk.org/c/asterisk/+/12842|https://gerrit.asterisk.org/c/asterisk/+/12842]

By: Friendly Automation (friendly-automation) 2019-09-05 07:52:25.302-0500

Change 12840 merged by George Joseph:
AST-2019-004 - res_pjsip_t38.c: Add NULL checks before using session media

[https://gerrit.asterisk.org/c/asterisk/+/12840|https://gerrit.asterisk.org/c/asterisk/+/12840]

By: Friendly Automation (friendly-automation) 2019-09-05 11:44:29.457-0500

Change 12847 merged by George Joseph:
AST-2019-004 - res_pjsip_t38.c: Add NULL checks before using session media

[https://gerrit.asterisk.org/c/asterisk/+/12847|https://gerrit.asterisk.org/c/asterisk/+/12847]