Re: [FFmpeg-devel] [PATCH v2] Fix sdp size check on fmtp integer parameters
On Mon, Apr 01, 2019 at 04:45:38PM +0200, 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, _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) {
This test only works if LONG_MAX > INT_MAX otherwise it will not
detect out of range values
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
signature.asc
Description: PGP signature
___
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, _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 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, _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".
