[Asterisk-code-review] pjproject: clone sdp in sip timer handling to protect against NAT fixup (asterisk[16])

[Michael Neuhauser]

Michael Neuhauser has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/14621 )


Change subject: pjproject: clone sdp in sip timer handling to protect against NAT fixup
......................................................................

pjproject: clone sdp in sip timer handling to protect against NAT fixup

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.
Solution: in pjproject/source/pjsip/src/pjsip-ua/sip_timer.c:timer_cb(),
modify a cloned copy of the pjmedia_sdp_stream, not the stored original
to protect it from changes (as is done in
pjproject/source/pjsip/src/pjsip-ua/sip_inv.c when calling
pjsip_create_sdp_body()).

ASTERISK-28973
Reported-by: Michael Neuhauser

Change-Id: I272ac22436076596e06aa51b9fa23fd1c7734a0e
---
A third-party/pjproject/patches/0060-clone-sdp-for-sip-timer-refresh-invite.patch
1 file changed, 12 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/21/14621/1

diff --git a/third-party/pjproject/patches/0060-clone-sdp-for-sip-timer-refresh-invite.patch b/third-party/pjproject/patches/0060-clone-sdp-for-sip-timer-refresh-invite.patch
new file mode 100644
index 0000000..7c367ec
--- /dev/null
+++ b/third-party/pjproject/patches/0060-clone-sdp-for-sip-timer-refresh-invite.patch
@@ -0,0 +1,12 @@
+diff -ur source.orig/pjsip/src/pjsip-ua/sip_timer.c source/pjsip/src/pjsip-ua/sip_timer.c
+--- source.orig/pjsip/src/pjsip-ua/sip_timer.c	2018-06-19 12:00:33.000000000 +0200
++++ source/pjsip/src/pjsip-ua/sip_timer.c	2020-06-30 15:55:51.501891714 +0200
+@@ -412,7 +412,7 @@
+ 		status = pjmedia_sdp_neg_get_neg_local(inv->neg, &offer);
+ 	    if (status == PJ_SUCCESS) {
+ 		status = pjsip_create_sdp_body(tdata->pool, 
+-					(pjmedia_sdp_session*)offer, &body);
++					pjmedia_sdp_session_clone(tdata->pool, offer), &body);
+ 		tdata->msg->body = body;
+ 	    }
+ 	}

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14621
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I272ac22436076596e06aa51b9fa23fd1c7734a0e
Gerrit-Change-Number: 14621
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Neuhauser <mike at firmix.at>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200630/cd285689/attachment.html>