From 9ea93c4c50a8d6285ee11789b21bafaa0d6117d9 Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Sun, 29 Jun 2014 17:32:33 +0000 Subject: [PATCH] Avoid buffer over-read on null cipher AEAD In the defined AEAD modes, SRTP packets must always be encrypted and authenticated, but SRTCP packets may be only authenticated. It's possible, therefore, for us to end up in `srtp_protect_aead()` without the `sec_serv_conf` bit being set. We should just ignore this and encrypt the RTP packet anyway. What we are doing instead is encrypting the packet anyway, but setting `enc_start` to NULL first. This causes `aad_len` to underflow which will cause us to over-read in `cipher_set_aad()`. If we could get past that, we would try to read and write memory starting at 0x0 down in `cipher_encrypt()`. This commit causes us to not check the `sec_serv_conf` bit and never set `enc_start` to NULL in `srtp_protect_aead()`. `srtp_unprotect_aead()` does not contain a similar error. --- libs/srtp/srtp/srtp.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/libs/srtp/srtp/srtp.c b/libs/srtp/srtp/srtp.c index 50d4c7a4af..43bf574bde 100644 --- a/libs/srtp/srtp/srtp.c +++ b/libs/srtp/srtp/srtp.c @@ -878,22 +878,16 @@ srtp_protect_aead (srtp_ctx_t *ctx, srtp_stream_ctx_t *stream, * encrypted - the encrypted portion starts after the rtp header * extension, if present; otherwise, it starts after the last csrc, * if any are present - * - * if we're not providing confidentiality, set enc_start to NULL */ - if (stream->rtp_services & sec_serv_conf) { - enc_start = (uint32_t*)hdr + uint32s_in_rtp_header + hdr->cc; - 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)); - } else { - enc_start = NULL; - } + enc_start = (uint32_t*)hdr + uint32s_in_rtp_header + hdr->cc; + 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)); /* * estimate the packet index using the start of the replay window