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
........

Merged revisions 378592 from http://svn.asterisk.org/svn/asterisk/branches/11


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@378593 65c4cc65-6c06-0410-ace0-fbb531ad65f3

1 files changed, 12 insertions, 3 deletions

diff --git a/res/res_srtp.c b/res/res_srtp.c
index b9499f8f1..549a1eaeb 100644
--- a/res/res_srtp.c
+++ b/res/res_srtp.c
@@ -366,7 +366,8 @@ tryagain:
 
 			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,15 +384,23 @@ tryagain:
 					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 ) {
 		if ((srtp->warned >= 10) && !((srtp->warned - 10) % 100)) {
 			ast_log(AST_LOG_WARNING, "SRTP unprotect failed with: %s %d\n", srtp_errstr(res), srtp->warned);