[FFmpeg-devel,v8] Fix integer parameters size check in SDP fmtp line

Submitted by Olivier Maignial on July 24, 2019, 8:20 a.m.

Details

Message ID 1563956414-11659-1-git-send-email-olivier.maignial@smile.fr
State New
Headers show

Commit Message

Olivier Maignial July 24, 2019, 8:20 a.m.

Comments

Michael Niedermayer July 25, 2019, 9:34 p.m.
On Wed, Jul 24, 2019 at 10:20:14AM +0200, Olivier Maignial wrote:
> === 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://<ip>/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 <olivier.maignial@smile.fr>
> ---
> Changes v7 --> v8:
>     Indroduced a per parameter validity range 

thanks, yes this should resolve the issue


[...]
Olivier Maignial Aug. 8, 2019, 1:39 p.m.
Hello here,

Just a mail to ping this patch

Have a nice day,

Olivier

On Thu, Jul 25, 2019 at 11:34 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Wed, Jul 24, 2019 at 10:20:14AM +0200, Olivier Maignial wrote:
> > === 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://<ip>/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 <olivier.maignial@smile.fr>
> > ---
> > Changes v7 --> v8:
> >     Indroduced a per parameter validity range
>
> thanks, yes this should resolve the issue
>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Complexity theory is the science of finding the exact solution to an
> approximation. Benchmarking OTOH is finding an approximation of the exact
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Michael Niedermayer Aug. 9, 2019, 9:52 p.m.
On Thu, Aug 08, 2019 at 03:39:00PM +0200, Olivier MAIGNIAL wrote:
> Hello here,
> 
> Just a mail to ping this patch

will apply

thanks

[...]

Patch hide | download patch | download mbox

=== 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://<ip>/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 <olivier.maignial@smile.fr>
---
Changes v7 --> v8:
    Indroduced a per parameter validity range 

 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 4f70599..08e5b98 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)