From c29d81e73641d0f58e6f058a095e18b99769dac1 Mon Sep 17 00:00:00 2001 From: Olivier Maignial Date: Wed, 24 Jul 2019 10:20:14 +0200 Subject: [PATCH] avformat/rtpdec_mpeg4: Fix integer parameters size check in SDP fmtp line === PROBLEM === I was trying to record h264 + aac streams from an RTSP server to mp4 file. using this command line: ffmpeg -v verbose -y -i "rtsp:///my_resources" -codec copy -bsf:a aac_adtstoasc test.mp4 FFmpeg then fail to record audio and output this logs: [rtsp @ 0xcda1f0] The profile-level-id field size is invalid (40) [rtsp @ 0xcda1f0] Error parsing AU headers ... [rtsp @ 0xcda1f0] Could not find codec parameters for stream 1 (Audio: aac, 48000 Hz, 1 channels): unspecified sample format In SDP provided by my RTSP server I had this fmtp line: a=fmtp:98 streamType=5; profile-level-id=40; mode=AAC-hbr; config=1188; sizeLength=13; indexLength=3; indexDeltaLength=3; In FFmpeg code, I found a check introduced by commit 24130234cd9dd733116d17b724ea4c8e12ce097a. It disallows values greater than 32 for fmtp line parameters. RFC-4566 (SDP: Session Description Protocol) do not give any limit of size on interger parameters given in an fmtp line. However, In RFC-6416 (RTP Payload Format for MPEG-4 Audio/Visual Streams) give examples of "profile-level-id" values for AAC, up to 55. === FIX === As each parameter may have its own min and max values I propose to introduce a range for each parameter. For this patch I used RFC-3640 and ISO/IEC 14496-1 as reference for validity ranges. This patch fix my problem and I now can record my RTSP AAC stream to mp4. It has passed the full fate tests suite sucessfully. Signed-off-by: Olivier Maignial Signed-off-by: Michael Niedermayer --- libavformat/rtpdec_mpeg4.c | 45 ++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c index 4f70599084..08e5b982c1 100644 --- a/libavformat/rtpdec_mpeg4.c +++ b/libavformat/rtpdec_mpeg4.c @@ -70,6 +70,12 @@ typedef struct AttrNameMap { const char *str; uint16_t type; uint32_t offset; + + /** Range for integer values */ + struct Range { + int min; + int max; + } range; } AttrNameMap; /* All known fmtp parameters and the corresponding RTPAttrTypeEnum */ @@ -77,18 +83,24 @@ typedef struct AttrNameMap { #define ATTR_NAME_TYPE_STR 1 static const AttrNameMap attr_names[] = { { "SizeLength", ATTR_NAME_TYPE_INT, - offsetof(PayloadContext, sizelength) }, + offsetof(PayloadContext, sizelength), + {0, 32} }, // SizeLength number of bits used to encode AU-size integer value { "IndexLength", ATTR_NAME_TYPE_INT, - offsetof(PayloadContext, indexlength) }, + offsetof(PayloadContext, indexlength), + {0, 32} }, // IndexLength number of bits used to encode AU-Index integer value { "IndexDeltaLength", ATTR_NAME_TYPE_INT, - offsetof(PayloadContext, indexdeltalength) }, + offsetof(PayloadContext, indexdeltalength), + {0, 32} }, // IndexDeltaLength number of bits to encode AU-Index-delta integer value { "profile-level-id", ATTR_NAME_TYPE_INT, - offsetof(PayloadContext, profile_level_id) }, + offsetof(PayloadContext, profile_level_id), + {INT32_MIN, INT32_MAX} }, // It differs depending on StreamType { "StreamType", ATTR_NAME_TYPE_INT, - offsetof(PayloadContext, streamtype) }, + offsetof(PayloadContext, streamtype), + {0x00, 0x3F} }, // Values from ISO/IEC 14496-1, 'StreamType Values' table { "mode", ATTR_NAME_TYPE_STR, - offsetof(PayloadContext, mode) }, - { NULL, -1, -1 }, + offsetof(PayloadContext, mode), + {0} }, + { NULL, -1, -1, {0} }, }; static void close_context(PayloadContext *data) @@ -289,15 +301,24 @@ static int parse_fmtp(AVFormatContext *s, for (i = 0; attr_names[i].str; ++i) { if (!av_strcasecmp(attr, attr_names[i].str)) { if (attr_names[i].type == ATTR_NAME_TYPE_INT) { - int val = atoi(value); - if (val > 32) { + char *end_ptr = NULL; + long long int val = strtoll(value, &end_ptr, 10); + if (end_ptr == value || end_ptr[0] != '\0') { av_log(s, AV_LOG_ERROR, - "The %s field size is invalid (%d)\n", - attr, val); + "The %s field value is not a valid number: %s\n", + attr, value); return AVERROR_INVALIDDATA; } + if (val < attr_names[i].range.min || + val > attr_names[i].range.max) { + av_log(s, AV_LOG_ERROR, + "fmtp field %s should be in range [%d,%d] (provided value: %lld)", + attr, attr_names[i].range.min, attr_names[i].range.max, val); + return AVERROR_INVALIDDATA; + } + *(int *)((char *)data+ - attr_names[i].offset) = val; + attr_names[i].offset) = (int) val; } else if (attr_names[i].type == ATTR_NAME_TYPE_STR) { char *val = av_strdup(value); if (!val) -- GitLab