chan_iax2: Fix crash due to negative length frame lengths.

chan_iax2 has several code paths where a frame's data length
is calculated by subtraction. On some paths, there is a check
for negative length. One of these paths is missing this check,
and on this path, it is possible for the result to be negative,
leading to a crash as a result of memory operations using the
bogus length.

Add a check to capture this off-nominal case. This will log
the appropriate warnings as in other cases and prevent a crash.
Also update the log messages to be clearer.

Resolves: #1707

diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index 1ddf66fe3d7fa82e6c519f1e64397ad595ccf106..c9aab2d20e8d80aa64657650310f5e25c2632d42 100644 (file)
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -10145,7 +10145,7 @@ static int socket_process_meta(int packet_len, struct ast_iax2_meta_hdr *meta, s
                                                iaxs[fr->callno]->last = fr->ts;
                                }
                        } else {
-                               ast_log(LOG_WARNING, "Datalen < 0?\n");
+                               ast_log(LOG_ERROR, "Dropping malformed frame (datalen %d?)\n", f.datalen);
                        }
                }
                ast_mutex_unlock(&iaxsl[fr->callno]);
@@ -12017,6 +12017,12 @@ immediatedial:
                        return 1;
                }
                f.datalen = res - sizeof(*vh);
+               if (f.datalen < 0) {
+                       ast_log(LOG_ERROR, "Dropping malformed frame (datalen %d?)\n", f.datalen);
+                       ast_variables_destroy(ies.vars);
+                       ast_mutex_unlock(&iaxsl[fr->callno]);
+                       return 1;
+               }
                if (f.datalen)
                        f.data.ptr = thread->buf + sizeof(*vh);
                else
@@ -12046,7 +12052,7 @@ immediatedial:
                }
                f.datalen = res - sizeof(struct ast_iax2_mini_hdr);
                if (f.datalen < 0) {
-                       ast_log(LOG_WARNING, "Datalen < 0?\n");
+                       ast_log(LOG_ERROR, "Dropping malformed frame (datalen %d?)\n", f.datalen);
                        ast_variables_destroy(ies.vars);
                        ast_mutex_unlock(&iaxsl[fr->callno]);
                        return 1;
@@ -12150,6 +12156,7 @@ immediatedial:
                        ast_frame_byteswap_be(&f);
        } else
                f.samples = 0;
+
        iax_frame_wrap(fr, &f);
 
        /* If this is our most recent packet, use it as our basis for timestamping */

diff --git a/channels/iax2/parser.c b/channels/iax2/parser.c
index 18d209d34bd7eaef420a5ec5d4d811f013beeddb..dfe8de866b10410d3db4d4fd4b6df826bd142882 100644 (file)
--- a/channels/iax2/parser.c
+++ b/channels/iax2/parser.c
@@ -1210,6 +1210,7 @@ void iax_frame_wrap(struct iax_frame *fr, struct ast_frame *f)
        if (fr->af.datalen) {
                size_t copy_len = fr->af.datalen;
                if (copy_len > fr->afdatalen) {
+                       ast_assert(fr->af.datalen >= 0); /* Length should never be negative */
                        ast_log(LOG_ERROR, "Losing frame data because destination buffer size '%d' bytes not big enough for '%d' bytes in the frame\n",
                                (int) fr->afdatalen, (int) fr->af.datalen);
                        copy_len = fr->afdatalen;
@@ -1230,6 +1231,8 @@ struct iax_frame *iax_frame_new(int direction, int datalen, unsigned int cacheab
 {
        struct iax_frame *fr;
 
+       ast_assert(datalen >= 0); /* Length should never be negative */
+
 #if !defined(NO_FRAME_CACHE)
        if (cacheable) {
                struct iax_frames *iax_frames;