diff mbox series

[FFmpeg-devel,v2,2/2] avformat: add data_size for ff_hex_to_data()

Message ID 1620385216-31085-2-git-send-email-lance.lmwang@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v2,1/2] avfilter/dnn/dnn_backend_tf: fix cross library usage | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Lance Wang May 7, 2021, 11 a.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

This prevents OOM in case of data buffer size is insufficient.

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/hls.c          | 2 +-
 libavformat/internal.h     | 6 ++++--
 libavformat/rtpdec_latm.c  | 4 ++--
 libavformat/rtpdec_mpeg4.c | 4 ++--
 libavformat/utils.c        | 7 +++++--
 5 files changed, 14 insertions(+), 9 deletions(-)

Comments

James Almer May 7, 2021, 12:40 p.m. UTC | #1 
On 5/7/2021 8:00 AM, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> This prevents OOM in case of data buffer size is insufficient.
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>   libavformat/hls.c          | 2 +-
>   libavformat/internal.h     | 6 ++++--
>   libavformat/rtpdec_latm.c  | 4 ++--
>   libavformat/rtpdec_mpeg4.c | 4 ++--
>   libavformat/utils.c        | 7 +++++--
>   5 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 584f658..c7f9f06 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -800,7 +800,7 @@ static int parse_playlist(HLSContext *c, const char *url,
>               if (!strcmp(info.method, "SAMPLE-AES"))
>                   key_type = KEY_SAMPLE_AES;
>               if (!strncmp(info.iv, "0x", 2) || !strncmp(info.iv, "0X", 2)) {
> -                ff_hex_to_data(iv, info.iv + 2);
> +                ff_hex_to_data(iv, sizeof(iv), info.iv + 2);
>                   has_iv = 1;
>               }
>               av_strlcpy(key, info.uri, sizeof(key));
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index 7d0eab4..e0e625f 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -397,10 +397,12 @@ char *ff_data_to_hex(char *buf, const uint8_t *src, int size, int lowercase);
>    * digits is ignored.
>    *
>    * @param data if non-null, the parsed data is written to this pointer
> + * @param data_size the data buffer size
>    * @param p the string to parse
> - * @return the number of bytes written (or to be written, if data is null)
> + * @return the number of bytes written (or to be written, if data is null),
> + *         or a negative value in case data buffer size is insufficient.

If it can fail now, then the callers should check for error codes.

>    */
> -int ff_hex_to_data(uint8_t *data, const char *p);
> +int ff_hex_to_data(uint8_t *data, int data_size, const char *p);
>   
>   /**
>    * Add packet to an AVFormatContext's packet_buffer list, determining its
> diff --git a/libavformat/rtpdec_latm.c b/libavformat/rtpdec_latm.c
> index 104a00a..c348cc8 100644
> --- a/libavformat/rtpdec_latm.c
> +++ b/libavformat/rtpdec_latm.c
> @@ -91,7 +91,7 @@ static int latm_parse_packet(AVFormatContext *ctx, PayloadContext *data,
>   
>   static int parse_fmtp_config(AVStream *st, const char *value)
>   {
> -    int len = ff_hex_to_data(NULL, value), i, ret = 0;
> +    int len = ff_hex_to_data(NULL, 0, value), i, ret = 0;
>       GetBitContext gb;
>       uint8_t *config;
>       int audio_mux_version, same_time_framing, num_programs, num_layers;
> @@ -100,7 +100,7 @@ static int parse_fmtp_config(AVStream *st, const char *value)
>       config = av_mallocz(len + AV_INPUT_BUFFER_PADDING_SIZE);
>       if (!config)
>           return AVERROR(ENOMEM);
> -    ff_hex_to_data(config, value);
> +    ff_hex_to_data(config, len, value);
>       init_get_bits(&gb, config, len*8);
>       audio_mux_version = get_bits(&gb, 1);
>       same_time_framing = get_bits(&gb, 1);
> diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
> index 34c7950..540192c 100644
> --- a/libavformat/rtpdec_mpeg4.c
> +++ b/libavformat/rtpdec_mpeg4.c
> @@ -112,11 +112,11 @@ static void close_context(PayloadContext *data)
>   static int parse_fmtp_config(AVCodecParameters *par, const char *value)
>   {
>       /* decode the hexa encoded parameter */
> -    int len = ff_hex_to_data(NULL, value), ret;
> +    int len = ff_hex_to_data(NULL, 0, value), ret;
>   
>       if ((ret = ff_alloc_extradata(par, len)) < 0)
>           return ret;
> -    ff_hex_to_data(par->extradata, value);
> +    ff_hex_to_data(par->extradata, par->extradata_size, value);
>       return 0;
>   }
>   
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 6c8b974..7085c28 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -4762,7 +4762,7 @@ char *ff_data_to_hex(char *buff, const uint8_t *src, int s, int lowercase)
>       return buff;
>   }
>   
> -int ff_hex_to_data(uint8_t *data, const char *p)
> +int ff_hex_to_data(uint8_t *data, int data_size, const char *p)
>   {
>       int c, len, v;
>   
> @@ -4781,8 +4781,11 @@ int ff_hex_to_data(uint8_t *data, const char *p)
>               break;
>           v = (v << 4) | c;
>           if (v & 0x100) {
> -            if (data)
> +            if (data) {
> +                if (len >= data_size)
> +                    return -1;

Don't propagate -1 as error. Use AVERROR(ENOMEM) or AVERROR(ERANGE) or 
something more adequate.

>                   data[len] = v;
> +            }
>               len++;
>               v = 1;
>           }
>
Lance Wang May 7, 2021, 3:21 p.m. UTC | #2 
On Fri, May 07, 2021 at 09:40:16AM -0300, James Almer wrote:
> On 5/7/2021 8:00 AM, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > This prevents OOM in case of data buffer size is insufficient.
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >   libavformat/hls.c          | 2 +-
> >   libavformat/internal.h     | 6 ++++--
> >   libavformat/rtpdec_latm.c  | 4 ++--
> >   libavformat/rtpdec_mpeg4.c | 4 ++--
> >   libavformat/utils.c        | 7 +++++--
> >   5 files changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > index 584f658..c7f9f06 100644
> > --- a/libavformat/hls.c
> > +++ b/libavformat/hls.c
> > @@ -800,7 +800,7 @@ static int parse_playlist(HLSContext *c, const char *url,
> >               if (!strcmp(info.method, "SAMPLE-AES"))
> >                   key_type = KEY_SAMPLE_AES;
> >               if (!strncmp(info.iv, "0x", 2) || !strncmp(info.iv, "0X", 2)) {
> > -                ff_hex_to_data(iv, info.iv + 2);
> > +                ff_hex_to_data(iv, sizeof(iv), info.iv + 2);
> >                   has_iv = 1;
> >               }
> >               av_strlcpy(key, info.uri, sizeof(key));
> > diff --git a/libavformat/internal.h b/libavformat/internal.h
> > index 7d0eab4..e0e625f 100644
> > --- a/libavformat/internal.h
> > +++ b/libavformat/internal.h
> > @@ -397,10 +397,12 @@ char *ff_data_to_hex(char *buf, const uint8_t *src, int size, int lowercase);
> >    * digits is ignored.
> >    *
> >    * @param data if non-null, the parsed data is written to this pointer
> > + * @param data_size the data buffer size
> >    * @param p the string to parse
> > - * @return the number of bytes written (or to be written, if data is null)
> > + * @return the number of bytes written (or to be written, if data is null),
> > + *         or a negative value in case data buffer size is insufficient.
> 
> If it can fail now, then the callers should check for error codes.

will add the error code check for the caller.

> 
> >    */
> > -int ff_hex_to_data(uint8_t *data, const char *p);
> > +int ff_hex_to_data(uint8_t *data, int data_size, const char *p);
> >   /**
> >    * Add packet to an AVFormatContext's packet_buffer list, determining its
> > diff --git a/libavformat/rtpdec_latm.c b/libavformat/rtpdec_latm.c
> > index 104a00a..c348cc8 100644
> > --- a/libavformat/rtpdec_latm.c
> > +++ b/libavformat/rtpdec_latm.c
> > @@ -91,7 +91,7 @@ static int latm_parse_packet(AVFormatContext *ctx, PayloadContext *data,
> >   static int parse_fmtp_config(AVStream *st, const char *value)
> >   {
> > -    int len = ff_hex_to_data(NULL, value), i, ret = 0;
> > +    int len = ff_hex_to_data(NULL, 0, value), i, ret = 0;
> >       GetBitContext gb;
> >       uint8_t *config;
> >       int audio_mux_version, same_time_framing, num_programs, num_layers;
> > @@ -100,7 +100,7 @@ static int parse_fmtp_config(AVStream *st, const char *value)
> >       config = av_mallocz(len + AV_INPUT_BUFFER_PADDING_SIZE);
> >       if (!config)
> >           return AVERROR(ENOMEM);
> > -    ff_hex_to_data(config, value);
> > +    ff_hex_to_data(config, len, value);
> >       init_get_bits(&gb, config, len*8);
> >       audio_mux_version = get_bits(&gb, 1);
> >       same_time_framing = get_bits(&gb, 1);
> > diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
> > index 34c7950..540192c 100644
> > --- a/libavformat/rtpdec_mpeg4.c
> > +++ b/libavformat/rtpdec_mpeg4.c
> > @@ -112,11 +112,11 @@ static void close_context(PayloadContext *data)
> >   static int parse_fmtp_config(AVCodecParameters *par, const char *value)
> >   {
> >       /* decode the hexa encoded parameter */
> > -    int len = ff_hex_to_data(NULL, value), ret;
> > +    int len = ff_hex_to_data(NULL, 0, value), ret;
> >       if ((ret = ff_alloc_extradata(par, len)) < 0)
> >           return ret;
> > -    ff_hex_to_data(par->extradata, value);
> > +    ff_hex_to_data(par->extradata, par->extradata_size, value);
> >       return 0;
> >   }
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index 6c8b974..7085c28 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -4762,7 +4762,7 @@ char *ff_data_to_hex(char *buff, const uint8_t *src, int s, int lowercase)
> >       return buff;
> >   }
> > -int ff_hex_to_data(uint8_t *data, const char *p)
> > +int ff_hex_to_data(uint8_t *data, int data_size, const char *p)
> >   {
> >       int c, len, v;
> > @@ -4781,8 +4781,11 @@ int ff_hex_to_data(uint8_t *data, const char *p)
> >               break;
> >           v = (v << 4) | c;
> >           if (v & 0x100) {
> > -            if (data)
> > +            if (data) {
> > +                if (len >= data_size)
> > +                    return -1;
> 
> Don't propagate -1 as error. Use AVERROR(ENOMEM) or AVERROR(ERANGE) or
> something more adequate.

will return AVERROR(ERANGE) for the error.

> 
> >                   data[len] = v;
> > +            }
> >               len++;
> >               v = 1;
> >           }
> > 
> 
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 584f658..c7f9f06 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -800,7 +800,7 @@  static int parse_playlist(HLSContext *c, const char *url,
             if (!strcmp(info.method, "SAMPLE-AES"))
                 key_type = KEY_SAMPLE_AES;
             if (!strncmp(info.iv, "0x", 2) || !strncmp(info.iv, "0X", 2)) {
-                ff_hex_to_data(iv, info.iv + 2);
+                ff_hex_to_data(iv, sizeof(iv), info.iv + 2);
                 has_iv = 1;
             }
             av_strlcpy(key, info.uri, sizeof(key));
diff --git a/libavformat/internal.h b/libavformat/internal.h
index 7d0eab4..e0e625f 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -397,10 +397,12 @@  char *ff_data_to_hex(char *buf, const uint8_t *src, int size, int lowercase);
  * digits is ignored.
  *
  * @param data if non-null, the parsed data is written to this pointer
+ * @param data_size the data buffer size
  * @param p the string to parse
- * @return the number of bytes written (or to be written, if data is null)
+ * @return the number of bytes written (or to be written, if data is null),
+ *         or a negative value in case data buffer size is insufficient.
  */
-int ff_hex_to_data(uint8_t *data, const char *p);
+int ff_hex_to_data(uint8_t *data, int data_size, const char *p);
 
 /**
  * Add packet to an AVFormatContext's packet_buffer list, determining its
diff --git a/libavformat/rtpdec_latm.c b/libavformat/rtpdec_latm.c
index 104a00a..c348cc8 100644
--- a/libavformat/rtpdec_latm.c
+++ b/libavformat/rtpdec_latm.c
@@ -91,7 +91,7 @@  static int latm_parse_packet(AVFormatContext *ctx, PayloadContext *data,
 
 static int parse_fmtp_config(AVStream *st, const char *value)
 {
-    int len = ff_hex_to_data(NULL, value), i, ret = 0;
+    int len = ff_hex_to_data(NULL, 0, value), i, ret = 0;
     GetBitContext gb;
     uint8_t *config;
     int audio_mux_version, same_time_framing, num_programs, num_layers;
@@ -100,7 +100,7 @@  static int parse_fmtp_config(AVStream *st, const char *value)
     config = av_mallocz(len + AV_INPUT_BUFFER_PADDING_SIZE);
     if (!config)
         return AVERROR(ENOMEM);
-    ff_hex_to_data(config, value);
+    ff_hex_to_data(config, len, value);
     init_get_bits(&gb, config, len*8);
     audio_mux_version = get_bits(&gb, 1);
     same_time_framing = get_bits(&gb, 1);
diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 34c7950..540192c 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -112,11 +112,11 @@  static void close_context(PayloadContext *data)
 static int parse_fmtp_config(AVCodecParameters *par, const char *value)
 {
     /* decode the hexa encoded parameter */
-    int len = ff_hex_to_data(NULL, value), ret;
+    int len = ff_hex_to_data(NULL, 0, value), ret;
 
     if ((ret = ff_alloc_extradata(par, len)) < 0)
         return ret;
-    ff_hex_to_data(par->extradata, value);
+    ff_hex_to_data(par->extradata, par->extradata_size, value);
     return 0;
 }
 
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 6c8b974..7085c28 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -4762,7 +4762,7 @@  char *ff_data_to_hex(char *buff, const uint8_t *src, int s, int lowercase)
     return buff;
 }
 
-int ff_hex_to_data(uint8_t *data, const char *p)
+int ff_hex_to_data(uint8_t *data, int data_size, const char *p)
 {
     int c, len, v;
 
@@ -4781,8 +4781,11 @@  int ff_hex_to_data(uint8_t *data, const char *p)
             break;
         v = (v << 4) | c;
         if (v & 0x100) {
-            if (data)
+            if (data) {
+                if (len >= data_size)
+                    return -1;
                 data[len] = v;
+            }
             len++;
             v = 1;
         }