sysmocom
/
asterisk
11
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

pjproject: clone sdp to protect against (nat) modifications 

PJSIP, UDP transport with external_media_address and session timers
enabled. Connected to SIP server that is not in local net. Asterisk
initiated the connection and is refreshing the session after 150s
(timeout 300s). The 2nd refresh-INVITE triggered by the pjsip timer has
a malformed IP address in its SDP (garbage string). This only happens
when the SDP is modified by the nat-code to replace the local IP address
with the configured external_media_address.
Analysis: the code to modify the SDP (in
res_pjsip_session.c:session_outgoing_nat_hook() and also (redundantly?)
in res_pjsip_sdp_rtp.c:change_outgoing_sdp_stream_media_address()) uses
the tdata->pool to allocate the replacement string. But the same
pjmedia_sdp_stream that was modified for the 1st refresh-INVITE is also
used for the 2nd refresh-INVITE (because it is stored in pjmedia's
pjmedia_sdp_neg structure). The problem is, that at that moment, the
tdata->pool that holds the stringified external_media_address from the
1. refresh-INVITE has long been reused for something else.
Fix by Sauw Ming of pjproject (see
https://github.com/pjsip/pjproject/pull/2476): the local, potentially
modified pjmedia_sdp_stream is cloned in
pjproject/source/pjsip/src/pjmedia/sip_neg.c:process_answer() and the
clone is stored, thereby detaching from the tdata->pool (which is only
released *after* process_answer())

ASTERISK-28973
Reported-by: Michael Neuhauser

Change-Id: I272ac22436076596e06aa51b9fa23fd1c7734a0e
This commit is contained in:
Michael Neuhauser 2020-06-30 17:40:41 +02:00 committed by Joshua Colp
parent 769a9611e7
commit 6482ab5bea
1 changed files with 28 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
third-party/pjproject/patches/0060-clone-sdp-for-sip-timer-refresh-invite.patch vendored Normal file
View File

@ -0,0 +1,28 @@
diff -ur source.orig/pjmedia/src/pjmedia/sdp_neg.c source/pjmedia/src/pjmedia/sdp_neg.c
--- source.orig/pjmedia/src/pjmedia/sdp_neg.c 2020-07-02 10:35:42.022459904 +0200
+++ source/pjmedia/src/pjmedia/sdp_neg.c 2020-07-02 10:33:24.996316867 +0200
@@ -906,7 +906,7 @@
* after receiving remote answer.
*/
static pj_status_t process_answer(pj_pool_t *pool,
- pjmedia_sdp_session *offer,
+ pjmedia_sdp_session *local_offer,
pjmedia_sdp_session *answer,
pj_bool_t allow_asym,
pjmedia_sdp_session **p_active)
@@ -914,10 +914,14 @@
unsigned omi = 0; /* Offer media index */
unsigned ami = 0; /* Answer media index */
pj_bool_t has_active = PJ_FALSE;
+ pjmedia_sdp_session *offer;
pj_status_t status;
/* Check arguments. */
- PJ_ASSERT_RETURN(pool && offer && answer && p_active, PJ_EINVAL);
+ PJ_ASSERT_RETURN(pool && local_offer && answer && p_active, PJ_EINVAL);
+
+ /* Duplicate local offer SDP. */
+ offer = pjmedia_sdp_session_clone(pool, local_offer);
/* Check that media count match between offer and answer */
// Ticket #527, different media count is allowed for more interoperability,