[Asterisk-code-review] res rtp asterisk.c: Fix uninitialized memory crash. (asterisk[certified/13.13])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/4670 )

Change subject: res_rtp_asterisk.c: Fix uninitialized memory crash.
......................................................................


res_rtp_asterisk.c: Fix uninitialized memory crash.

ast_rtp_remote_address_set() could pass an uninitialized 'us' parameter to
ast_ouraddrfor().  If ast_ouraddrfor() returns an error then the 'us'
parameter may not get initialized.  Thus when the code tries to save the
'us' parameter to the local address we could try to copy a ridiculous
sized memory buffer and segfault.

* Made pass an initialized 'us' parameter to ast_ouraddrfor().

* Optimized out the 'us' struct variable.

ASTERISK-26672 #close

Change-Id: I4acea5dcdf0813da2c7d3e11c2d6067d160d17dc
---
M res/res_rtp_asterisk.c
1 file changed, 12 insertions(+), 14 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index d271e72..1fe59d8 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -4974,31 +4974,31 @@
 static void ast_rtp_remote_address_set(struct ast_rtp_instance *instance, struct ast_sockaddr *addr)
 {
 	struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
-	struct ast_sockaddr local, us;
+	struct ast_sockaddr local;
 
+	ast_rtp_instance_get_local_address(instance, &local);
 	if (!ast_sockaddr_isnull(addr)) {
 		/* Update the local RTP address with what is being used */
-		ast_ouraddrfor(addr, &us);
-		ast_rtp_instance_get_local_address(instance, &local);
-		ast_sockaddr_set_port(&us, ast_sockaddr_port(&local));
-		ast_rtp_instance_set_local_address(instance, &us);
+		if (ast_ouraddrfor(addr, &local)) {
+			/* Failed to update our address so reuse old local address */
+			ast_rtp_instance_get_local_address(instance, &local);
+		} else {
+			ast_rtp_instance_set_local_address(instance, &local);
+		}
 	}
 
 	if (rtp->rtcp) {
 		ast_debug(1, "Setting RTCP address on RTP instance '%p'\n", instance);
 		ast_sockaddr_copy(&rtp->rtcp->them, addr);
 		if (!ast_sockaddr_isnull(addr)) {
-			ast_sockaddr_set_port(&rtp->rtcp->them,
-					      ast_sockaddr_port(addr) + 1);
-		}
+			ast_sockaddr_set_port(&rtp->rtcp->them, ast_sockaddr_port(addr) + 1);
 
-		if (!ast_sockaddr_isnull(addr)) {
 			/* Update the local RTCP address with what is being used */
-			ast_sockaddr_set_port(&us, ast_sockaddr_port(&local) + 1);
-			ast_sockaddr_copy(&rtp->rtcp->us, &us);
+			ast_sockaddr_set_port(&local, ast_sockaddr_port(&local) + 1);
+			ast_sockaddr_copy(&rtp->rtcp->us, &local);
 
 			ast_free(rtp->rtcp->local_addr_str);
-			rtp->rtcp->local_addr_str = ast_strdup(ast_sockaddr_stringify(&us));
+			rtp->rtcp->local_addr_str = ast_strdup(ast_sockaddr_stringify(&local));
 		}
 	}
 
@@ -5008,8 +5008,6 @@
 		rtp->strict_rtp_state = STRICT_RTP_LEARN;
 		rtp_learning_seq_init(&rtp->rtp_source_learn, rtp->seqno);
 	}
-
-	return;
 }
 
 /*! \brief Write t140 redundacy frame

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I4acea5dcdf0813da2c7d3e11c2d6067d160d17dc
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>