Avoid buffer-overflow on short RTCP/SRTCP packets

In `srtp_unprotect_rtcp()` we are not validating that the packet
length is as long as the minimum required.  This would cause
`enc_octet_len` to underflow, which would cause us to try to decrypt
data past the end of the packet in memory -- a buffer over-read and
buffer overflow.

In `srtp_protect_rtcp()`, we were similarly not validating the packet
length.  Here we were also polluting the address of the SRTCP
encrypted flag and index (the `trailer`), causing us to write one word
to a bogus memory address before getting to the encryption where we
would also overflow.

In this commit we add checks to appropriately validate the RTCP/SRTCP
packet lengths.

`srtp_unprotect_rtcp_aead()` (but not protect) did correctly validate
the packet length; this check would now be redundant as the check in
`srtcp_unprotect_rtcp()` will also run first, so it has been removed.
This commit is contained in:
Travis Cross 2014-06-29 18:42:29 +00:00
parent 9ea93c4c50
commit aa4261d11f

View File

@ -2342,12 +2342,6 @@ srtp_unprotect_rtcp_aead (srtp_t ctx, srtp_stream_ctx_t *stream,
/* get tag length from stream context */
tag_len = auth_get_tag_length(stream->rtcp_auth);
/* Validate packet length */
if (*pkt_octet_len < (octets_in_rtcp_header + tag_len +
sizeof(srtcp_trailer_t))) {
return err_status_bad_param;
}
/*
* set encryption start, encryption length, and trailer
*/
@ -2520,6 +2514,11 @@ srtp_protect_rtcp(srtp_t ctx, void *rtcp_hdr, int *pkt_octet_len) {
uint32_t seq_num;
/* we assume the hdr is 32-bit aligned to start */
/* check the packet length - it must at least contain a full header */
if (*pkt_octet_len < octets_in_rtcp_header)
return err_status_bad_param;
/*
* look up ssrc in srtp_stream list, and process the packet with
* the appropriate stream. if we haven't seen this stream before,
@ -2753,6 +2752,16 @@ srtp_unprotect_rtcp(srtp_t ctx, void *srtcp_hdr, int *pkt_octet_len) {
}
}
/* get tag length from stream context */
tag_len = auth_get_tag_length(stream->rtcp_auth);
/* check the packet length - it must contain at least a full RTCP
header, an auth tag (if applicable), and the SRTCP encrypted flag
and 31-bit index value */
if (*pkt_octet_len < (octets_in_rtcp_header + tag_len +
sizeof(srtcp_trailer_t)))
return err_status_bad_param;
/*
* Check if this is an AEAD stream (GCM mode). If so, then dispatch
* the request to our AEAD handler.
@ -2765,9 +2774,6 @@ srtp_unprotect_rtcp(srtp_t ctx, void *srtcp_hdr, int *pkt_octet_len) {
sec_serv_confidentiality = stream->rtcp_services == sec_serv_conf ||
stream->rtcp_services == sec_serv_conf_and_auth;
/* get tag length from stream context */
tag_len = auth_get_tag_length(stream->rtcp_auth);
/*
* set encryption start, encryption length, and trailer
*/