res_srtp: Prevent a crash from occurring due to srtp_create failures in srtp_create
authorJonathan Rose <jrose@digium.com>
Fri, 4 Jan 2013 23:14:54 +0000 (23:14 +0000)
committerJonathan Rose <jrose@digium.com>
Fri, 4 Jan 2013 23:14:54 +0000 (23:14 +0000)
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

res/res_srtp.c

index b9499f8..549a1ea 100644 (file)
@@ -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);