diff mbox

[FFmpeg-devel] Fix sdp size check on fmtp integer parameters

Message ID 1551102890-952-1-git-send-email-olivier.maignial@smile.fr
State Superseded
Headers show

Commit Message

Olivier Maignial Feb. 25, 2019, 1:54 p.m. UTC 
---
 libavformat/rtpdec_mpeg4.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Michael Niedermayer Feb. 27, 2019, 3:14 p.m. UTC | #1 
On Mon, Feb 25, 2019 at 02:54:50PM +0100, Olivier Maignial wrote:
> ---
>  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) {

i belive strtol can fail with other errno values


>                          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;

this is not needed, though it does no harm if the intend is to make it
clear that the type is converted intentionally here

[...]
Nicolas George Feb. 27, 2019, 3:17 p.m. UTC | #2 
Michael Niedermayer (12019-02-27):
> >                  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) {
> 
> i belive strtol can fail with other errno values

And errno is not reset in case of successful completion. It needs to be
cleared beforehand.

To check if a conversion error happened, the middle argument is the
proper tool.

Regards,
diff mbox

Patch

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)