[asterisk-commits] jrose: branch 11 r378592 - in /branches/11: ./ res/res_srtp.c

[SVN commits to the Asterisk project]

Author: jrose
Date: Fri Jan  4 17:04:59 2013
New Revision: 378592

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=378592
Log:
res_srtp: Prevent a crash from occurring due to srtp_create failures in srtp_create

Under some circumstances, libsrtp's srtp_create function deallocates memory that
it wasn't initially responsible for allocating. Because we weren't initially
aware of this behavior, this memory was still used in spite of being unallocated
during the course of the srtp_unprotect function. A while back I made a patch
which would set this value to NULL, but that exposed a possible condition where
we would then try to check a member of the struct which would cause a segfault.
In order to address these problems, ast_srtp_unprotect will now set an error value
when it ends without a valid SRTP session which will result in the caller of
srtp_unprotect observing this error and hanging up the relevant channel instead of
trying to keep using the invalid session address.

(closes issue ASTERISK-20499)
Reported by: Tootai
Review: https://reviewboard.asterisk.org/r/2228/diff/#index_header
........

Merged revisions 378591 from http://svn.asterisk.org/svn/asterisk/branches/1.8

Modified:
    branches/11/   (props changed)
    branches/11/res/res_srtp.c

Propchange: branches/11/
------------------------------------------------------------------------------
--- branch-1.8-merged (original)
+++ branch-1.8-merged Fri Jan  4 17:04:59 2013
@@ -1,1 +1,1 @@
-/branches/1.8:1-378147,378164,378356,378375,378427,378455-378456,378486,378514,378554
+/branches/1.8:1-378147,378164,378356,378375,378427,378455-378456,378486,378514,378554,378591

Modified: branches/11/res/res_srtp.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/res/res_srtp.c?view=diff&rev=378592&r1=378591&r2=378592
==============================================================================
--- branches/11/res/res_srtp.c (original)
+++ branches/11/res/res_srtp.c Fri Jan  4 17:04:59 2013
@@ -366,7 +366,8 @@
 
 			ast_debug(5, "SRTP try to re-create\n");
 			if (policy) {
-				if (srtp_create(&srtp->session, &policy->sp) == err_status_ok) {
+				int res_srtp_create = srtp_create(&srtp->session, &policy->sp);
+				if (res_srtp_create == err_status_ok) {
 					ast_debug(5, "SRTP re-created with first policy\n");
 					ao2_t_ref(policy, -1, "Unreffing first policy for re-creating srtp session");
 
@@ -383,13 +384,21 @@
 					retry++;
 					ao2_iterator_destroy(&it);
 					goto tryagain;
-				} else {
-					srtp->session = NULL;
 				}
+				ast_log(LOG_ERROR, "SRTP session could not be re-created after unprotect failure: %s\n", srtp_errstr(res_srtp_create));
+
+				/* If srtp_create() fails with a previously alloced session, it will have been dealloced before returning. */
+				srtp->session = NULL;
+
 				ao2_t_ref(policy, -1, "Unreffing first policy after srtp_create failed");
 			}
 			ao2_iterator_destroy(&it);
 		}
+	}
+
+	if (!srtp->session) {
+		errno = EINVAL;
+		return -1;
 	}
 
 	if (res != err_status_ok && res != err_status_replay_fail ) {