]> git.ipfire.org Git - thirdparty/freeswitch.git/commitdiff
Prevent buffer overflow from untrusted RTP/SRTP lengths
authorTravis Cross <tc@traviscross.com>
Sun, 29 Jun 2014 01:10:29 +0000 (01:10 +0000)
committerTravis Cross <tc@traviscross.com>
Mon, 30 Jun 2014 19:00:35 +0000 (19:00 +0000)
When computing the start address of the RTP data to encrypt or SRTP
data to decrypt (`enc_start`), we are using `hdr->cc` (the CSRC
count), which is untrusted data from the packet, and the length field
of an RTP header extension, which is also untrusted and unchecked data
from the packet.

This value then pollutes our calculation of how much data we'll be
encrypting or decrypting (`enc_octet_len`), possibly causing us to
underflow.

We'll then call `cipher_encrypt()` or `cipher_decrypt()` with these
two values, causing us to read from and write to arbitrary addresses
in memory.

(In the AEAD functions, we'd also pollute `aad_len`, which would cause
us to read undefined memory in `cipher_set_aad`.)

This commit adds checks to verify that the `enc_start` we calculate is
sane based on the actual packet length.

libs/srtp/srtp/srtp.c

index 328319f3df2017de90f95fd730ebc77cefb93bd6..50d4c7a4af516b8513bce633fd7cb9d7eeda541d 100644 (file)
@@ -887,6 +887,8 @@ srtp_protect_aead (srtp_ctx_t *ctx, srtp_stream_ctx_t *stream,
             srtp_hdr_xtnd_t *xtn_hdr = (srtp_hdr_xtnd_t*)enc_start;
             enc_start += (ntohs(xtn_hdr->length) + 1);
         }
+        if (!(enc_start < (uint32_t*)hdr + *pkt_octet_len))
+            return err_status_parse_err;
         enc_octet_len = (unsigned int)(*pkt_octet_len - 
                        ((enc_start - (uint32_t*)hdr) << 2));
     } else {
@@ -1015,6 +1017,8 @@ srtp_unprotect_aead (srtp_ctx_t *ctx, srtp_stream_ctx_t *stream, int delta,
         srtp_hdr_xtnd_t *xtn_hdr = (srtp_hdr_xtnd_t*)enc_start;
         enc_start += (ntohs(xtn_hdr->length) + 1);
     }
+    if (!(enc_start < (uint32_t*)hdr + *pkt_octet_len))
+        return err_status_parse_err;
     /*
      * We pass the tag down to the cipher when doing GCM mode 
      */
@@ -1229,6 +1233,8 @@ srtp_unprotect_aead (srtp_ctx_t *ctx, srtp_stream_ctx_t *stream, int delta,
      if (hdr->x == 1) {
        srtp_hdr_xtnd_t *xtn_hdr = (srtp_hdr_xtnd_t *)enc_start;
        enc_start += (ntohs(xtn_hdr->length) + 1);
+       if (!(enc_start < (uint32_t*)hdr + *pkt_octet_len))
+         return err_status_parse_err;
      }
      enc_octet_len = (unsigned int)(*pkt_octet_len 
                                    - ((enc_start - (uint32_t *)hdr) << 2));
@@ -1510,6 +1516,8 @@ srtp_unprotect(srtp_ctx_t *ctx, void *srtp_hdr, int *pkt_octet_len) {
       srtp_hdr_xtnd_t *xtn_hdr = (srtp_hdr_xtnd_t *)enc_start;
       enc_start += (ntohs(xtn_hdr->length) + 1);
     }  
+    if (!(enc_start < (uint32_t*)hdr + *pkt_octet_len))
+      return err_status_parse_err;
     enc_octet_len = (uint32_t)(*pkt_octet_len - tag_len 
                               - ((enc_start - (uint32_t *)hdr) << 2));
   } else {