[Asterisk-code-review] res pjsip refer: Fix crash from a REFER and BYE collision. (asterisk[certified/13.1])

Kevin Harwell asteriskteam at digium.com
Tue Jun 2 12:41:33 CDT 2015


Kevin Harwell has uploaded a new change for review.

  https://gerrit.asterisk.org/566

Change subject: res_pjsip_refer: Fix crash from a REFER and BYE collision.
......................................................................

res_pjsip_refer: Fix crash from a REFER and BYE collision.

Analyzing a one-off crash on a busy system showed that processing a REFER
request had a NULL session channel pointer.  The only way I can think of
that could cause this is if an outgoing BYE transaction overlapped the
incoming REFER transaction in a collision.  Asterisk sends a BYE while the
phone sends a REFER to complete an attended transfer.

* Made check the session channel pointer before processing an incoming
REFER request in res_pjsip_refer.

* Fixed similar crash potential for res_pjsip supplement incoming request
processing for res_pjsip_sdp_rtp INFO, res_pjsip_caller_id INVITE/UPDATE,
res_pjsip_messaging MESSAGE, and res_pjsip_send_to_voicemail REFER
messages.

* Made res_pjsip_messaging respond to a message body too large with a 413
instead of ignoring it.

ASTERISK-24700 #close
Reported by: Zane Conkle

Review: https://reviewboard.asterisk.org/r/4417/
........

Merged revisions 431898 from http://svn.asterisk.org/svn/asterisk/branches/13

Change-Id: I57878adc0846dd942a699ad36dcec9cba5e57994
---
M res/res_pjsip_caller_id.c
M res/res_pjsip_messaging.c
M res/res_pjsip_refer.c
M res/res_pjsip_sdp_rtp.c
M res/res_pjsip_send_to_voicemail.c
5 files changed, 31 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/66/566/1

diff --git a/res/res_pjsip_caller_id.c b/res/res_pjsip_caller_id.c
index c3757e0..dc595c4 100644
--- a/res/res_pjsip_caller_id.c
+++ b/res/res_pjsip_caller_id.c
@@ -361,7 +361,7 @@
 		if (!session->endpoint->id.self.number.valid) {
 			set_id_from_from(rdata, &session->id);
 		}
-	} else {
+	} else if (session->channel) {
 		/* Reinvite. Check for changes to the ID and queue a connected line
 		 * update if necessary
 		 */
diff --git a/res/res_pjsip_messaging.c b/res/res_pjsip_messaging.c
index c125864..dab70ca 100644
--- a/res/res_pjsip_messaging.c
+++ b/res/res_pjsip_messaging.c
@@ -692,9 +692,13 @@
 	char buf[MAX_BODY_SIZE];
 	enum pjsip_status_code code;
 	struct ast_frame f;
-
 	pjsip_dialog *dlg = session->inv_session->dlg;
 	pjsip_transaction *tsx = pjsip_rdata_get_tsx(rdata);
+
+	if (!session->channel) {
+		send_response(rdata, PJSIP_SC_NOT_FOUND, dlg, tsx);
+		return 0;
+	}
 
 	if ((code = check_content_type(rdata)) != PJSIP_SC_OK) {
 		send_response(rdata, code, dlg, tsx);
@@ -703,6 +707,7 @@
 
 	if (print_body(rdata, buf, sizeof(buf)-1) < 1) {
 		/* invalid body size */
+		send_response(rdata, PJSIP_SC_REQUEST_ENTITY_TOO_LARGE, dlg, tsx);
 		return 0;
 	}
 
diff --git a/res/res_pjsip_refer.c b/res/res_pjsip_refer.c
index 88f135c..771ab11 100644
--- a/res/res_pjsip_refer.c
+++ b/res/res_pjsip_refer.c
@@ -418,7 +418,7 @@
 	struct refer_attended *attended = obj;
 
 	ao2_cleanup(attended->transferer);
-	ast_channel_unref(attended->transferer_chan);
+	ast_channel_cleanup(attended->transferer_chan);
 	ao2_cleanup(attended->transferer_second);
 	ao2_cleanup(attended->progress);
 }
@@ -667,7 +667,7 @@
 
 		return 200;
 	} else {
-		const char *context = (session->channel ? pbx_builtin_getvar_helper(session->channel, "TRANSFER_CONTEXT") : "");
+		const char *context = pbx_builtin_getvar_helper(session->channel, "TRANSFER_CONTEXT");
 		struct refer_blind refer = { 0, };
 
 		if (ast_strlen_zero(context)) {
@@ -710,10 +710,6 @@
 	const char *context;
 	char exten[AST_MAX_EXTENSION];
 	struct refer_blind refer = { 0, };
-
-	if (!session->channel) {
-		return 404;
-	}
 
 	/* If no explicit transfer context has been provided use their configured context */
 	context = pbx_builtin_getvar_helper(session->channel, "TRANSFER_CONTEXT");
@@ -886,6 +882,14 @@
 	static const pj_str_t str_refer_to = { "Refer-To", 8 };
 	static const pj_str_t str_replaces = { "Replaces", 8 };
 
+	if (!session->channel) {
+		/* No channel to refer.  Likely because the call was just hung up. */
+		pjsip_dlg_respond(session->inv_session->dlg, rdata, 404, NULL, NULL, NULL);
+		ast_debug(3, "Received a REFER on a session with no channel from endpoint '%s'.\n",
+			ast_sorcery_object_get_id(session->endpoint));
+		return 0;
+	}
+
 	if (!session->endpoint->allowtransfer) {
 		pjsip_dlg_respond(session->inv_session->dlg, rdata, 603, NULL, NULL, NULL);
 		ast_log(LOG_WARNING, "Endpoint %s transfer attempt blocked due to configuration\n",
diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index 1994db7..a85d1e8 100644
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -1274,15 +1274,18 @@
 
 static int video_info_incoming_request(struct ast_sip_session *session, struct pjsip_rx_data *rdata)
 {
-	struct pjsip_transaction *tsx = pjsip_rdata_get_tsx(rdata);
+	struct pjsip_transaction *tsx;
 	pjsip_tx_data *tdata;
 
-	if (!ast_sip_is_content_type(&rdata->msg_info.msg->body->content_type,
-				     "application",
-				     "media_control+xml")) {
+	if (!session->channel
+		|| !ast_sip_is_content_type(&rdata->msg_info.msg->body->content_type,
+			"application",
+			"media_control+xml")) {
 		return 0;
 	}
 
+	tsx = pjsip_rdata_get_tsx(rdata);
+
 	ast_queue_control(session->channel, AST_CONTROL_VIDUPDATE);
 
 	if (pjsip_dlg_create_response(session->inv_session->dlg, rdata, 200, NULL, &tdata) == PJ_SUCCESS) {
diff --git a/res/res_pjsip_send_to_voicemail.c b/res/res_pjsip_send_to_voicemail.c
index 97f55d3..3a57aea 100644
--- a/res/res_pjsip_send_to_voicemail.c
+++ b/res/res_pjsip_send_to_voicemail.c
@@ -119,13 +119,17 @@
 
 static int handle_incoming_request(struct ast_sip_session *session, struct pjsip_rx_data *rdata)
 {
-
 	struct ast_datastore *sip_session_datastore;
 	struct ast_channel *other_party;
+	int has_feature;
+	int has_reason;
 
-	int has_feature = has_call_feature(rdata);
-	int has_reason = has_diversion_reason(rdata);
+	if (!session->channel) {
+		return 0;
+	}
 
+	has_feature = has_call_feature(rdata);
+	has_reason = has_diversion_reason(rdata);
 	if (!has_feature && !has_reason) {
 		/* If we don't have a call feature or diversion reason or if
 		   it's not a feature this module is related to then there

-- 
To view, visit https://gerrit.asterisk.org/566
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I57878adc0846dd942a699ad36dcec9cba5e57994
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.1
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>



More information about the asterisk-code-review mailing list