From de26a4b6993ff3dc91f17d110326736c96bfc9ec Mon Sep 17 00:00:00 2001 From: Ivan Kovtunov Date: Fri, 4 May 2012 22:31:46 +0300 Subject: rtpdec_h264: Add input size checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes crashes if given too short data packets. Signed-off-by: Martin Storsjö --- libavformat/rtpdec_h264.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'libavformat/rtpdec_h264.c') diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c index 32a57d3ec7..51447f9703 100644 --- a/libavformat/rtpdec_h264.c +++ b/libavformat/rtpdec_h264.c @@ -173,11 +173,18 @@ static int h264_handle_packet(AVFormatContext *ctx, const uint8_t * buf, int len, int flags) { - uint8_t nal = buf[0]; - uint8_t type = (nal & 0x1f); + uint8_t nal; + uint8_t type; int result= 0; uint8_t start_sequence[] = { 0, 0, 0, 1 }; + if (!len) { + av_log(ctx, AV_LOG_ERROR, "Empty H264 RTP packet\n"); + return AVERROR_INVALIDDATA; + } + nal = buf[0]; + type = nal & 0x1f; + #ifdef DEBUG assert(data); assert(data->cookie == MAGIC_COOKIE); @@ -271,7 +278,7 @@ static int h264_handle_packet(AVFormatContext *ctx, case 28: // FU-A (fragmented nal) buf++; len--; // skip the fu_indicator - { + if (len > 1) { // these are the same as above, we just redo them here for clarity... uint8_t fu_indicator = nal; uint8_t fu_header = *buf; // read the fu_header. @@ -302,6 +309,9 @@ static int h264_handle_packet(AVFormatContext *ctx, av_new_packet(pkt, len); memcpy(pkt->data, buf, len); } + } else { + av_log(ctx, AV_LOG_ERROR, "Too short data for FU-A H264 RTP packet\n"); + result = AVERROR_INVALIDDATA; } break; -- cgit v1.2.3 From 5245adb963e35660a70115c437cd67ea0c398885 Mon Sep 17 00:00:00 2001 From: Martin Storsjö Date: Fri, 4 May 2012 22:45:11 +0300 Subject: rtpdec_h264: Check the available data length before reading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes sure the length is checked for STAP-A type packets. Signed-off-by: Martin Storsjö --- libavformat/rtpdec_h264.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'libavformat/rtpdec_h264.c') diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c index 51447f9703..8b56ada397 100644 --- a/libavformat/rtpdec_h264.c +++ b/libavformat/rtpdec_h264.c @@ -218,7 +218,7 @@ static int h264_handle_packet(AVFormatContext *ctx, const uint8_t *src= buf; int src_len= len; - do { + while (src_len > 2) { uint16_t nal_size = AV_RB16(src); // this going to be a problem if unaligned (can it be?) // consume the length of the aggregate... @@ -252,7 +252,7 @@ static int h264_handle_packet(AVFormatContext *ctx, if (src_len < 0) av_log(ctx, AV_LOG_ERROR, "Consumed more bytes than we got! (%d)\n", src_len); - } while (src_len > 2); // because there could be rtp padding.. + } if(pass==0) { // now we know the total size of the packet (with the start sequences added) -- cgit v1.2.3 From b7b7354c336a0e3df8b6e7ca8baebe8750ed0f45 Mon Sep 17 00:00:00 2001 From: Martin Storsjö Date: Sat, 5 May 2012 00:29:15 +0300 Subject: rtpdec_h264: Return proper error codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Storsjö --- libavformat/rtpdec_h264.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'libavformat/rtpdec_h264.c') diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c index 8b56ada397..f3793f5ec6 100644 --- a/libavformat/rtpdec_h264.c +++ b/libavformat/rtpdec_h264.c @@ -272,7 +272,7 @@ static int h264_handle_packet(AVFormatContext *ctx, av_log(ctx, AV_LOG_ERROR, "Unhandled type (%d) (See RFC for implementation details\n", type); - result= -1; + result = AVERROR(ENOSYS); break; case 28: // FU-A (fragmented nal) @@ -319,7 +319,7 @@ static int h264_handle_packet(AVFormatContext *ctx, case 31: // undefined default: av_log(ctx, AV_LOG_ERROR, "Undefined type (%d)", type); - result= -1; + result = AVERROR_INVALIDDATA; break; } -- cgit v1.2.3 From 5a571d324129ce367584ad9d92aae1d286f389a2 Mon Sep 17 00:00:00 2001 From: Martin Storsjö Date: Fri, 4 May 2012 23:49:45 +0300 Subject: rtpdec_h264: Remove useless memory corruption checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Storsjö --- libavformat/rtpdec_h264.c | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) (limited to 'libavformat/rtpdec_h264.c') diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c index f3793f5ec6..eb20397811 100644 --- a/libavformat/rtpdec_h264.c +++ b/libavformat/rtpdec_h264.c @@ -53,8 +53,6 @@ RTP/H264 specific private data. */ struct PayloadContext { - unsigned long cookie; ///< sanity check, to make sure we get the pointer we're expecting. - //sdp setup parameters uint8_t profile_idc; ///< from the sdp setup parameters. uint8_t profile_iop; ///< from the sdp setup parameters. @@ -65,9 +63,6 @@ struct PayloadContext { #endif }; -#define MAGIC_COOKIE (0xdeadbeef) ///< Cookie for the extradata; to verify we are what we think we are, and that we haven't been freed. -#define DEAD_COOKIE (0xdeaddead) ///< Cookie for the extradata; once it is freed. - /* ---------------- private code */ static int sdp_parse_fmtp_config_h264(AVStream * stream, PayloadContext * h264_data, @@ -187,7 +182,6 @@ static int h264_handle_packet(AVFormatContext *ctx, #ifdef DEBUG assert(data); - assert(data->cookie == MAGIC_COOKIE); #endif assert(buf); @@ -331,15 +325,7 @@ static int h264_handle_packet(AVFormatContext *ctx, /* ---------------- public code */ static PayloadContext *h264_new_context(void) { - PayloadContext *data = - av_mallocz(sizeof(PayloadContext) + - FF_INPUT_BUFFER_PADDING_SIZE); - - if (data) { - data->cookie = MAGIC_COOKIE; - } - - return data; + return av_mallocz(sizeof(PayloadContext) + FF_INPUT_BUFFER_PADDING_SIZE); } static void h264_free_context(PayloadContext *data) @@ -354,13 +340,6 @@ static void h264_free_context(PayloadContext *data) } #endif - assert(data); - assert(data->cookie == MAGIC_COOKIE); - - // avoid stale pointers (assert) - data->cookie = DEAD_COOKIE; - - // and clear out this... av_free(data); } @@ -376,7 +355,6 @@ static int parse_h264_sdp_line(AVFormatContext *s, int st_index, stream = s->streams[st_index]; codec = stream->codec; - assert(h264_data->cookie == MAGIC_COOKIE); if (av_strstart(p, "framesize:", &p)) { char buf1[50]; -- cgit v1.2.3 From 8d43b8b8e84495deeebc1f01a2035ba76ba24a7a Mon Sep 17 00:00:00 2001 From: Martin Storsjö Date: Fri, 4 May 2012 23:46:18 +0300 Subject: rtpdec_h264: Remove outdated/useless/incorrect comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RTCP is handled elsewhere, not in the depacketizer for an individual format. Signed-off-by: Martin Storsjö --- libavformat/rtpdec_h264.c | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) (limited to 'libavformat/rtpdec_h264.c') diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c index eb20397811..d98be07366 100644 --- a/libavformat/rtpdec_h264.c +++ b/libavformat/rtpdec_h264.c @@ -30,10 +30,6 @@ * Single Nal Unit Mode (0), or * Non-Interleaved Mode (1). It currently does not support * Interleaved Mode (2). (This requires implementing STAP-B, MTAP16, MTAP24, FU-B packet types) - * - * @note TODO: - * 1) RTCP sender reports for udp streams are required.. - * */ #include "libavutil/base64.h" @@ -49,21 +45,17 @@ #include "rtpdec.h" #include "rtpdec_formats.h" -/** - RTP/H264 specific private data. -*/ struct PayloadContext { //sdp setup parameters - uint8_t profile_idc; ///< from the sdp setup parameters. - uint8_t profile_iop; ///< from the sdp setup parameters. - uint8_t level_idc; ///< from the sdp setup parameters. - int packetization_mode; ///< from the sdp setup parameters. + uint8_t profile_idc; + uint8_t profile_iop; + uint8_t level_idc; + int packetization_mode; #ifdef DEBUG int packet_types_received[32]; #endif }; -/* ---------------- private code */ static int sdp_parse_fmtp_config_h264(AVStream * stream, PayloadContext * h264_data, char *attr, char *value) @@ -99,7 +91,6 @@ static int sdp_parse_fmtp_config_h264(AVStream * stream, buffer[0] = value[4]; buffer[1] = value[5]; level_idc = strtol(buffer, NULL, 16); - // set the parameters... av_log(codec, AV_LOG_DEBUG, "RTP Profile IDC: %x Profile IOP: %x Level: %x\n", profile_idc, profile_iop, level_idc); @@ -136,7 +127,6 @@ static int sdp_parse_fmtp_config_h264(AVStream * stream, { if(codec->extradata_size) { - // av_realloc? memcpy(dest, codec->extradata, codec->extradata_size); av_free(codec->extradata); } @@ -213,7 +203,7 @@ static int h264_handle_packet(AVFormatContext *ctx, int src_len= len; while (src_len > 2) { - uint16_t nal_size = AV_RB16(src); // this going to be a problem if unaligned (can it be?) + uint16_t nal_size = AV_RB16(src); // consume the length of the aggregate... src += 2; @@ -275,7 +265,7 @@ static int h264_handle_packet(AVFormatContext *ctx, if (len > 1) { // these are the same as above, we just redo them here for clarity... uint8_t fu_indicator = nal; - uint8_t fu_header = *buf; // read the fu_header. + uint8_t fu_header = *buf; uint8_t start_bit = fu_header >> 7; // uint8_t end_bit = (fu_header & 0x40) >> 6; uint8_t nal_type = (fu_header & 0x1f); @@ -322,7 +312,6 @@ static int h264_handle_packet(AVFormatContext *ctx, return result; } -/* ---------------- public code */ static PayloadContext *h264_new_context(void) { return av_mallocz(sizeof(PayloadContext) + FF_INPUT_BUFFER_PADDING_SIZE); @@ -380,12 +369,9 @@ static int parse_h264_sdp_line(AVFormatContext *s, int st_index, // could use this if we wanted. } - return 0; // keep processing it the normal way... + return 0; } -/** -This is the structure for expanding on the dynamic rtp protocols (makes everything static. yay!) -*/ RTPDynamicProtocolHandler ff_h264_dynamic_handler = { .enc_name = "H264", .codec_type = AVMEDIA_TYPE_VIDEO, -- cgit v1.2.3 From 44f99fe0f5e688833cb671efbaa0beb6b74c9389 Mon Sep 17 00:00:00 2001 From: Martin Storsjö Date: Fri, 4 May 2012 23:53:10 +0300 Subject: rtpdec_h264: Remove a useless ifdef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit assert is a no-op if DEBUG isn't defined. Signed-off-by: Martin Storsjö --- libavformat/rtpdec_h264.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'libavformat/rtpdec_h264.c') diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c index d98be07366..399ca47ff7 100644 --- a/libavformat/rtpdec_h264.c +++ b/libavformat/rtpdec_h264.c @@ -170,9 +170,7 @@ static int h264_handle_packet(AVFormatContext *ctx, nal = buf[0]; type = nal & 0x1f; -#ifdef DEBUG assert(data); -#endif assert(buf); if (type >= 1 && type <= 23) -- cgit v1.2.3