[FFmpeg-devel] [PATCH v2] Fix sdp size check on fmtp integer parameters
RFC-4566 do not give any limit of size on interger parameters given in fmtp
line.
By reading some more RFCs it is possible to find examples where some integers
parameters are greater than 32 (see RFC-6416, 7.4)
Instead I propose to check just check the eventual integer overflow.
Using INT_MIN and INT_MAX ensure that it will work whatever the size of int
given by compiler
Signed-off-by: Olivier Maignial
---
Changes v1 -> v2:
- Removed line break at end of 'if' line before brace
- Added Signed-Off-By line
libavformat/rtpdec_mpeg4.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 994ab49..14caa0a 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -289,15 +289,23 @@ 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 int val = strtol(value, &end_ptr, 10);
+if (value[0] == '\n' || end_ptr[0] != '\0') {
av_log(s, AV_LOG_ERROR,
- "The %s field size is invalid (%d)\n",
+ "The %s field value is not a number (%s)\n",
+ attr, value);
+return AVERROR_INVALIDDATA;
+}
+
+if (val > INT_MAX || val < INT_MIN) {
+av_log(s, AV_LOG_ERROR,
+ "The %s field size is invalid (%ld)\n",
attr, 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)
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] Fix sdp size check on fmtp integer parameters
Hello all
Just a reminder for this patch
On Mon, Apr 1, 2019 at 4:45 PM Olivier Maignial
wrote:
> RFC-4566 do not give any limit of size on interger parameters given in
> fmtp line.
> By reading some more RFCs it is possible to find examples where some
> integers parameters are greater than 32 (see RFC-6416, 7.4)
>
> Instead I propose to check just check the eventual integer overflow.
> Using INT_MIN and INT_MAX ensure that it will work whatever the size of
> int given by compiler
>
> Signed-off-by: Olivier Maignial
> ---
>
> Changes v1 -> v2:
> - Removed line break at end of 'if' line before brace
> - Added Signed-Off-By line
>
> libavformat/rtpdec_mpeg4.c | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
> index 994ab49..14caa0a 100644
> --- a/libavformat/rtpdec_mpeg4.c
> +++ b/libavformat/rtpdec_mpeg4.c
> @@ -289,15 +289,23 @@ 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 int val = strtol(value, &end_ptr, 10);
> +if (value[0] == '\n' || end_ptr[0] != '\0') {
> av_log(s, AV_LOG_ERROR,
> - "The %s field size is invalid (%d)\n",
> + "The %s field value is not a number
> (%s)\n",
> + attr, value);
> +return AVERROR_INVALIDDATA;
> +}
> +
> +if (val > INT_MAX || val < INT_MIN) {
> +av_log(s, AV_LOG_ERROR,
> + "The %s field size is invalid (%ld)\n",
> attr, 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)
> --
> 2.7.4
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3] Fix sdp size check on fmtp integer parameters
RFC-4566 do not give any limit of size on interger parameters given in fmtp
line.
By reading some more RFCs it is possible to find examples where some integers
parameters are greater than 32 (see RFC-6416, 7.4)
Instead I propose to check just check the eventual integer overflow.
Using INT_MIN and INT_MAX ensure that it will work whatever the size of int
given by compiler
Signed-off-by: Olivier Maignial
---
Changes v2 -> v3:
Fix over/underflow checking in case of sizeof(int) == sizeof(long)
If MAX/MIN_INT == MAX/MIN_LONG overflow would not be detected by just checking
value is in range [MAX_INT,MIN_INT].
In case of over/underflow strtol return MAX/MIN_LONG and set errno to ERANGE.
As MAX/MIN_LONG are valid values, the only way to detect over/underflow is to
check errno.
libavformat/rtpdec_mpeg4.c | 28 +++-
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 4f70599..d40cb5a 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -289,15 +289,33 @@ 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;
+errno = 0;
+long int val = strtol(value, &end_ptr, 10);
+if (value[0] == '\n' || 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 number (%s)\n",
+ attr, value);
return AVERROR_INVALIDDATA;
}
+if ((val == LONG_MAX && errno == ERANGE) ||
+val > INT_MAX) {
+av_log(s, AV_LOG_ERROR,
+ "Value of field %s overflow maximum integer
value.\n",
+ attr);
+return AVERROR_INVALIDDATA;
+}
+if ((val == LONG_MIN && errno == ERANGE) ||
+val < INT_MIN)
+{
+av_log(s, AV_LOG_ERROR,
+ "Value of field %s underflow minimum integer
value.\n",
+ attr);
+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)
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3] Fix sdp size check on fmtp integer parameters
Hello,
Just a reminder about this patch :)
On Fri, Apr 19, 2019 at 3:00 PM Olivier Maignial
wrote:
> RFC-4566 do not give any limit of size on interger parameters given in
> fmtp line.
> By reading some more RFCs it is possible to find examples where some
> integers parameters are greater than 32 (see RFC-6416, 7.4)
>
> Instead I propose to check just check the eventual integer overflow.
> Using INT_MIN and INT_MAX ensure that it will work whatever the size of
> int given by compiler
>
> Signed-off-by: Olivier Maignial
> ---
>
> Changes v2 -> v3:
> Fix over/underflow checking in case of sizeof(int) == sizeof(long)
>
> If MAX/MIN_INT == MAX/MIN_LONG overflow would not be detected by just
> checking value is in range [MAX_INT,MIN_INT].
>
> In case of over/underflow strtol return MAX/MIN_LONG and set errno to
> ERANGE.
> As MAX/MIN_LONG are valid values, the only way to detect over/underflow is
> to check errno.
>
> libavformat/rtpdec_mpeg4.c | 28 +++-
> 1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
> index 4f70599..d40cb5a 100644
> --- a/libavformat/rtpdec_mpeg4.c
> +++ b/libavformat/rtpdec_mpeg4.c
> @@ -289,15 +289,33 @@ 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;
> +errno = 0;
> +long int val = strtol(value, &end_ptr, 10);
> +if (value[0] == '\n' || 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 number
> (%s)\n",
> + attr, value);
> return AVERROR_INVALIDDATA;
> }
> +if ((val == LONG_MAX && errno == ERANGE) ||
> +val > INT_MAX) {
> +av_log(s, AV_LOG_ERROR,
> + "Value of field %s overflow maximum
> integer value.\n",
> + attr);
> +return AVERROR_INVALIDDATA;
> +}
> +if ((val == LONG_MIN && errno == ERANGE) ||
> +val < INT_MIN)
> +{
> +av_log(s, AV_LOG_ERROR,
> + "Value of field %s underflow minimum
> integer value.\n",
> + attr);
> +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)
> --
> 2.7.4
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v4] 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 disallow values greater than 32
for fmtp line parameters.
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.
Furthermore, RFC-4566 (SDP: Session Description Protocol) do not give any limit
of size on interger parameters given in fmtp line.
=== FIX ===
Instead of prohibit values over 32, I propose to check the possible integer
overflow.
The use of strtol allow to check the string validity and the possible overflow.
Using INT_MIN, LONG_MIN, INT_MAX and LON_MAX definitions ensure that it will
work whatever the size of int/long given by compiler.
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
---
Changes v3->v4
- Rebased my patch on master
- Updated comit log to provide better explanation of the problem
- Re-passed fate tests on master
libavformat/rtpdec_mpeg4.c | 28 +++-
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 4f70599..d40cb5a 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -289,15 +289,33 @@ 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;
+errno = 0;
+long int val = strtol(value, &end_ptr, 10);
+if (value[0] == '\n' || 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 number (%s)\n",
+ attr, value);
return AVERROR_INVALIDDATA;
}
+if ((val == LONG_MAX && errno == ERANGE) ||
+val > INT_MAX) {
+av_log(s, AV_LOG_ERROR,
+ "Value of field %s overflow maximum integer
value.\n",
+ attr);
+return AVERROR_INVALIDDATA;
+}
+if ((val == LONG_MIN && errno == ERANGE) ||
+val < INT_MIN)
+{
+av_log(s, AV_LOG_ERROR,
+ "Value of field %s underflow minimum integer
value.\n",
+ attr);
+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)
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v5] 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.
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.
Furthermore, RFC-4566 (SDP: Session Description Protocol) do not give any limit
of size on interger parameters given in fmtp line.
=== FIX ===
Instead of prohibit values over 32, I propose to check the possible integer
overflow.
The use of strtol allow to check the string validity and the possible overflow.
To store and check return of strtol I use "long int" type and LONG_MIN/MAX
definitions despite differences on 32/64bits platforms.
It is consistent with the strtol man page, and it is the only way to check if
overflow or underflow is detected by strtol.
As the value is later checked against INT32_MIN and INT32_MAX, the behavior
will finnaly be the same on both type of platform.
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
---
Changes V4 -> V5:
- Check value against INT32_MAX/MIN instead of INT_MAX/MIN which can be
defferent depending on platform
libavformat/rtpdec_mpeg4.c | 28 +++-
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 4f70599..cf35afb 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -289,15 +289,33 @@ 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;
+errno = 0;
+long int val = strtol(value, &end_ptr, 10);
+if (value[0] == '\n' || 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 number (%s)\n",
+ attr, value);
return AVERROR_INVALIDDATA;
}
+if ((val == LONG_MAX && errno == ERANGE) ||
+val > INT32_MAX) {
+av_log(s, AV_LOG_ERROR,
+ "Value of field %s overflows maximum integer
value.\n",
+ attr);
+return AVERROR_INVALIDDATA;
+}
+if ((val == LONG_MIN && errno == ERANGE) ||
+val < INT32_MIN)
+{
+av_log(s, AV_LOG_ERROR,
+ "Value of field %s underflows minimum integer
value.\n",
+ attr);
+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)
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v5] Fix integer parameters size check in SDP fmtp line
Hello,
Just a reminder about this patch
If you need something else to validate it, please let me know
Olivier
On Wed, May 22, 2019 at 6:10 PM 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:///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.
> 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.
> Furthermore, RFC-4566 (SDP: Session Description Protocol) do not give any
> limit of size on interger parameters given in fmtp line.
>
> === FIX ===
>
> Instead of prohibit values over 32, I propose to check the possible
> integer overflow.
> The use of strtol allow to check the string validity and the possible
> overflow.
>
> To store and check return of strtol I use "long int" type and LONG_MIN/MAX
> definitions despite differences on 32/64bits platforms.
> It is consistent with the strtol man page, and it is the only way to check
> if overflow or underflow is detected by strtol.
> As the value is later checked against INT32_MIN and INT32_MAX, the
> behavior will finnaly be the same on both type of platform.
>
> 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
> ---
>
> Changes V4 -> V5:
> - Check value against INT32_MAX/MIN instead of INT_MAX/MIN which can
> be defferent depending on platform
>
> libavformat/rtpdec_mpeg4.c | 28 +++-
> 1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
> index 4f70599..cf35afb 100644
> --- a/libavformat/rtpdec_mpeg4.c
> +++ b/libavformat/rtpdec_mpeg4.c
> @@ -289,15 +289,33 @@ 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;
> +errno = 0;
> +long int val = strtol(value, &end_ptr, 10);
> +if (value[0] == '\n' || 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 number
> (%s)\n",
> + attr, value);
> return AVERROR_INVALIDDATA;
> }
> +if ((val == LONG_MAX && errno == ERANGE) ||
> +val > INT32_MAX) {
> +av_log(s, AV_LOG_ERROR,
> + "Value of field %s overflows maximum
> integer value.\n",
> + attr);
> +return AVERROR_INVALIDDATA;
> +}
> +if ((val == LONG_MIN && errno == ERANGE) ||
> +val < INT32_MIN)
> +{
> +av_log(s, AV_LOG_ERROR,
> + "Value of field %s underflows minimum
> integer value.\n",
> + attr);
> +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)
> --
> 2.7.4
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v6] 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.
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.
Furthermore, RFC-4566 (SDP: Session Description Protocol) do not give any limit
of size on interger parameters given in fmtp line.
=== FIX ===
Instead of prohibit values over 32, I propose to check the possible integer
overflow.
The use of strtol allow to check the string validity and the possible overflow.
Value is then checked against INT32_MIN and INT32_MAX. Using INT32_MIN/MAX
ensure to have the same behavior on 32 or 64 bits platforms.
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
---
Changes V5 -> V6:
- Simplify code
libavformat/rtpdec_mpeg4.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 4f70599..9c4f8a1 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -289,15 +289,20 @@ 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;
+errno = 0;
+long int val = strtol(value, &end_ptr, 10);
+if (end_ptr == value || end_ptr[0] != '\0' ||
+errno == ERANGE ||
+val < INT32_MIN || val > INT32_MAX) {
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, or
overflows int32: %s\n",
+ attr, value);
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)
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v6] Fix integer parameters size check in SDP fmtp line
Hello here!
A simple ping about this patch
If you have any question, feel free to ask!
Regards,
Olivier
On Wed, Jun 19, 2019 at 3:38 PM 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:///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.
> 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.
> Furthermore, RFC-4566 (SDP: Session Description Protocol) do not give any
> limit of size on interger parameters given in fmtp line.
>
> === FIX ===
>
> Instead of prohibit values over 32, I propose to check the possible
> integer overflow.
> The use of strtol allow to check the string validity and the possible
> overflow.
> Value is then checked against INT32_MIN and INT32_MAX. Using INT32_MIN/MAX
> ensure to have the same behavior on 32 or 64 bits platforms.
>
> 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
> ---
>
> Changes V5 -> V6:
> - Simplify code
>
> libavformat/rtpdec_mpeg4.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
> index 4f70599..9c4f8a1 100644
> --- a/libavformat/rtpdec_mpeg4.c
> +++ b/libavformat/rtpdec_mpeg4.c
> @@ -289,15 +289,20 @@ 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;
> +errno = 0;
> +long int val = strtol(value, &end_ptr, 10);
> +if (end_ptr == value || end_ptr[0] != '\0' ||
> +errno == ERANGE ||
> +val < INT32_MIN || val > INT32_MAX) {
> 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,
> or overflows int32: %s\n",
> + attr, value);
> 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)
> --
> 2.7.4
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v6] Fix integer parameters size check in SDP fmtp line
Hello, thanks for review,
1) Can you give some reason about why we shouldn't use errno? I think it is
more clear to accept the full int32 range, even if min/max int32 values are
very unlikely to be used.
2) The RFC 4566 don't give any limit on fmtp parameters values. In addition
I can't find a spec that says fmtp integers parameters for AAC must be
positive. So I don't think we should limit values to positive integers here
as it would not be consistent to RFC.
On Sat, Jun 29, 2019 at 5:58 AM Reimar Döffinger
wrote:
> I don't think we should be using errno when avoidable, and it is avoidable
> here by disallowing min/max int32 values themselves. Or using strtoll.
> I'm also rather sceptical about allowing negative values here, does that
> make sense?
> Admittedly the type is set to just "int", but maybe it should be unsigned
> instead?
>
> On 28.06.2019, at 08:46, Olivier MAIGNIAL
> wrote:
>
> > Hello here!
> >
> > A simple ping about this patch
> > If you have any question, feel free to ask!
> >
> > Regards,
> > Olivier
> >
> > On Wed, Jun 19, 2019 at 3:38 PM Olivier Maignial <
> olivier.maign...@smile.fr>
> > 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:///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.
> >> 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.
> >> Furthermore, RFC-4566 (SDP: Session Description Protocol) do not give
> any
> >> limit of size on interger parameters given in fmtp line.
> >>
> >> === FIX ===
> >>
> >> Instead of prohibit values over 32, I propose to check the possible
> >> integer overflow.
> >> The use of strtol allow to check the string validity and the possible
> >> overflow.
> >> Value is then checked against INT32_MIN and INT32_MAX. Using
> INT32_MIN/MAX
> >> ensure to have the same behavior on 32 or 64 bits platforms.
> >>
> >> 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
> >> ---
> >>
> >> Changes V5 -> V6:
> >>- Simplify code
> >>
> >> libavformat/rtpdec_mpeg4.c | 15 ++-
> >> 1 file changed, 10 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
> >> index 4f70599..9c4f8a1 100644
> >> --- a/libavformat/rtpdec_mpeg4.c
> >> +++ b/libavformat/rtpdec_mpeg4.c
> >> @@ -289,15 +289,20 @@ 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;
> >> +errno = 0;
> >> +long int val = strtol(value, &end_ptr, 10);
> >> +if (end_ptr == value || end_ptr[0] != '\0' ||
> >> +errno == ERANGE ||
> >> +val < INT32_MIN || val > INT32_MAX) {
> >> av_log(s, AV_LOG_ERROR,
> >> - "The %s field size is invalid (%d)\n",
> >> - attr, val);
>
[FFmpeg-devel] [PATCH v6] 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.
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.
Furthermore, RFC-4566 (SDP: Session Description Protocol) do not give any limit
of size on interger parameters given in fmtp line.
=== FIX ===
Instead of prohibit values over 32, I propose to check the possible integer
overflow.
The use of strtoll allow to check the string validity and the possible overflow.
Value is then checked against INT32_MIN and INT32_MAX. Using INT32_MIN/MAX
ensure to have the same behavior on 32 or 64 bits platforms.
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
---
Changes V6 --> V7:
- Use long long int and strtoll. LLONG_MAX is always greather than
INT32_MIN while LONG_MAX can be equal to INT32_MAX. It allows to accept full
INT32 range.
- Avoid to use errno
libavformat/rtpdec_mpeg4.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 4f70599..f1cbedf 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -289,15 +289,18 @@ 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' ||
+val < INT32_MIN || val > INT32_MAX) {
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, or
overflows int32: %s\n",
+ attr, value);
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)
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v6] Fix integer parameters size check in SDP fmtp line
You are right, some values are expected to be <= 32.
However, it doesn't add a bug: Without my patch fail will occur at parsing
time; With my patch it will occur a bit later on check performed by
get_bits_long().
Maybe we can add a specific max value to each parameter to make logs more
accurate on what happens.
On Wed, Jul 17, 2019 at 10:28 PM Michael Niedermayer
wrote:
> On Fri, Jul 12, 2019 at 08:40:40AM +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:///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.
> > 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.
> > Furthermore, RFC-4566 (SDP: Session Description Protocol) do not give
> any limit of size on interger parameters given in fmtp line.
> >
> > === FIX ===
> >
> > Instead of prohibit values over 32, I propose to check the possible
> integer overflow.
> > The use of strtoll allow to check the string validity and the possible
> overflow.
> > Value is then checked against INT32_MIN and INT32_MAX. Using
> INT32_MIN/MAX ensure to have the same behavior on 32 or 64 bits platforms.
> >
> > 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
> > ---
> >
> > Changes V6 --> V7:
> > - Use long long int and strtoll. LLONG_MAX is always greather than
> INT32_MIN while LONG_MAX can be equal to INT32_MAX. It allows to accept
> full INT32 range.
> > - Avoid to use errno
> >
> > libavformat/rtpdec_mpeg4.c | 13 -
> > 1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
> > index 4f70599..f1cbedf 100644
> > --- a/libavformat/rtpdec_mpeg4.c
> > +++ b/libavformat/rtpdec_mpeg4.c
> > @@ -289,15 +289,18 @@ 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' ||
> > +val < INT32_MIN || val > INT32_MAX) {
> > 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, or overflows int32: %s\n",
> > + attr, value);
> > return AVERROR_INVALIDDATA;
> > }
> > +
> > *(int *)((char *)data+
> > -attr_names[i].offset) = val;
> > +attr_names[i].offset) = (int) val;
>
> Looking at this a bit deeper, this reads several specific values
> for example sizelength which is then used to read with get_bits_long()
> that is limited to 32bit. Maybe iam missing some check but if not
> that is adding a bug.
> I did not check what the valid range of the other cases is but
> they may have similar problems
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v8] 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
---
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_ttype;
uint32_toffset;
+
+/** 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)"
Re: [FFmpeg-devel] [PATCH v8] Fix integer parameters size check in SDP fmtp line
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 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:///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 > > --- > > 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-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] Fix sdp size check on fmtp integer parameters
---
libavformat/rtpdec_mpeg4.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 4f70599..f632ebf 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -289,15 +289,15 @@ 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) {
+long int val = strtol(value, NULL, 10);
+if (errno == ERANGE || val > INT_MAX || val < INT_MIN) {
av_log(s, AV_LOG_ERROR,
- "The %s field size is invalid (%d)\n",
+ "The %s field size is invalid (%ld)\n",
attr, 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)
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] Fix sdp size check on fmtp integer parameters
RFC-4566 do not give any limit of size on interger parameters given in fmtp
line.
By reading some more RFCs it is possible to find examples where some integers
parameters are greater than 32 (see RFC-6416, 7.4)
---
libavformat/rtpdec_mpeg4.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 994ab49..4b86f4a 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -289,15 +289,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 int val = strtol(value, &end_ptr, 10);
+if (value[0] == '\n' || end_ptr[0] != '\0')
+{
av_log(s, AV_LOG_ERROR,
- "The %s field size is invalid (%d)\n",
+ "The %s field value is not a number (%s)\n",
+ attr, value);
+return AVERROR_INVALIDDATA;
+}
+
+if (val > INT_MAX || val < INT_MIN) {
+av_log(s, AV_LOG_ERROR,
+ "The %s field size is invalid (%ld)\n",
attr, 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)
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] Fix sdp size check on fmtp integer parameters
RFC-4566 do not give any limit of size on interger parameters given in fmtp
line.
By reading some more RFCs it is possible to find examples where some integers
parameters are greater than 32 (see RFC-6416, 7.4)
Instead I propose to check just check the eventual integer overflow.
Using INT_MIN and INT_MAX ensure that it will work whatever the size of int
given by compiler
---
libavformat/rtpdec_mpeg4.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 994ab49..14caa0a 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -289,15 +289,23 @@ 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 int val = strtol(value, &end_ptr, 10);
+if (value[0] == '\n' || end_ptr[0] != '\0') {
av_log(s, AV_LOG_ERROR,
- "The %s field size is invalid (%d)\n",
+ "The %s field value is not a number (%s)\n",
+ attr, value);
+return AVERROR_INVALIDDATA;
+}
+
+if (val > INT_MAX || val < INT_MIN) {
+av_log(s, AV_LOG_ERROR,
+ "The %s field size is invalid (%ld)\n",
attr, 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)
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
