diff mbox

[FFmpeg-devel] avformat/http: Check for truncated buffers in http_connect()

Message ID 20170213124640.29060-1-michael@niedermayer.cc
State Accepted
Commit 8fa18e042ad2c078f759692f1db5629d16d70595
Headers show

Commit Message

Michael Niedermayer Feb. 13, 2017, 12:46 p.m. UTC
Reported-by: SleepProgger <security@gnutp.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/http.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Steven Liu Feb. 14, 2017, 1:39 a.m. UTC | #1
2017-02-13 20:46 GMT+08:00 Michael Niedermayer <michael@niedermayer.cc>:

> Reported-by: SleepProgger <security@gnutp.com>
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/http.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 944a6cf322..bd1be3f7bb 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -1011,6 +1011,7 @@ static int http_connect(URLContext *h, const char
> *path, const char *local_path,
>      int len = 0;
>      const char *method;
>      int send_expect_100 = 0;
> +    int ret;
>
>      /* send http header */
>      post = h->flags & AVIO_FLAG_WRITE;
> @@ -1107,7 +1108,7 @@ static int http_connect(URLContext *h, const char
> *path, const char *local_path,
>      if (s->headers)
>          av_strlcpy(headers + len, s->headers, sizeof(headers) - len);
>
> -    snprintf(s->buffer, sizeof(s->buffer),
> +    ret = snprintf(s->buffer, sizeof(s->buffer),
>               "%s %s HTTP/1.1\r\n"
>               "%s"
>               "%s"
> @@ -1123,6 +1124,14 @@ static int http_connect(URLContext *h, const char
> *path, const char *local_path,
>
>      av_log(h, AV_LOG_DEBUG, "request: %s\n", s->buffer);
>
> +    if (strlen(headers) + 1 == sizeof(headers) ||
> +        ret >= sizeof(s->buffer)) {
> +        av_log(h, AV_LOG_ERROR, "overlong headers\n");
> +        err = AVERROR(EINVAL);
> +        goto done;
> +    }
> +
> +
>      if ((err = ffurl_write(s->hd, s->buffer, strlen(s->buffer))) < 0)
>          goto done;
>
> --
> 2.11.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



LGTM,

BTW, what about give a initial value 0 when define the ‘ret’ variable?


Thanks
Michael Niedermayer Feb. 14, 2017, 3:04 a.m. UTC | #2
On Tue, Feb 14, 2017 at 09:39:08AM +0800, Steven Liu wrote:
> 2017-02-13 20:46 GMT+08:00 Michael Niedermayer <michael@niedermayer.cc>:
> 
> > Reported-by: SleepProgger <security@gnutp.com>
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavformat/http.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavformat/http.c b/libavformat/http.c
> > index 944a6cf322..bd1be3f7bb 100644
> > --- a/libavformat/http.c
> > +++ b/libavformat/http.c
> > @@ -1011,6 +1011,7 @@ static int http_connect(URLContext *h, const char
> > *path, const char *local_path,
> >      int len = 0;
> >      const char *method;
> >      int send_expect_100 = 0;
> > +    int ret;
> >
> >      /* send http header */
> >      post = h->flags & AVIO_FLAG_WRITE;
> > @@ -1107,7 +1108,7 @@ static int http_connect(URLContext *h, const char
> > *path, const char *local_path,
> >      if (s->headers)
> >          av_strlcpy(headers + len, s->headers, sizeof(headers) - len);
> >
> > -    snprintf(s->buffer, sizeof(s->buffer),
> > +    ret = snprintf(s->buffer, sizeof(s->buffer),
> >               "%s %s HTTP/1.1\r\n"
> >               "%s"
> >               "%s"
> > @@ -1123,6 +1124,14 @@ static int http_connect(URLContext *h, const char
> > *path, const char *local_path,
> >
> >      av_log(h, AV_LOG_DEBUG, "request: %s\n", s->buffer);
> >
> > +    if (strlen(headers) + 1 == sizeof(headers) ||
> > +        ret >= sizeof(s->buffer)) {
> > +        av_log(h, AV_LOG_ERROR, "overlong headers\n");
> > +        err = AVERROR(EINVAL);
> > +        goto done;
> > +    }
> > +
> > +
> >      if ((err = ffurl_write(s->hd, s->buffer, strlen(s->buffer))) < 0)
> >          goto done;
> >
> > --
> > 2.11.0
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> 
> 
> LGTM,

applied


> 
> BTW, what about give a initial value 0 when define the ‘ret’ variable?

it would result in warnings of being unneeded in some tools probably

thx

[...]
su.gao Feb. 14, 2017, 3:23 a.m. UTC | #3
I found that the box_length is  shorter  than the text_length, and there is no option to adapt(Equal).

So  I suggest ffmpeg.org can add a opption(like boxw_equal_textw) to adapt it !

the old source:
    box_w = FFMIN(width - 1 , max_text_line_w);
    box_h = FFMIN(height - 1, y + s->max_glyph_h);

the new source:
    if(0==s->boxw_equal_textw){
          box_w = FFMIN(width - 1 , max_text_line_w);
    }else{
          box_w =   (int)s->var_values[VAR_TEXT_W];   
    }
diff mbox

Patch

diff --git a/libavformat/http.c b/libavformat/http.c
index 944a6cf322..bd1be3f7bb 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -1011,6 +1011,7 @@  static int http_connect(URLContext *h, const char *path, const char *local_path,
     int len = 0;
     const char *method;
     int send_expect_100 = 0;
+    int ret;
 
     /* send http header */
     post = h->flags & AVIO_FLAG_WRITE;
@@ -1107,7 +1108,7 @@  static int http_connect(URLContext *h, const char *path, const char *local_path,
     if (s->headers)
         av_strlcpy(headers + len, s->headers, sizeof(headers) - len);
 
-    snprintf(s->buffer, sizeof(s->buffer),
+    ret = snprintf(s->buffer, sizeof(s->buffer),
              "%s %s HTTP/1.1\r\n"
              "%s"
              "%s"
@@ -1123,6 +1124,14 @@  static int http_connect(URLContext *h, const char *path, const char *local_path,
 
     av_log(h, AV_LOG_DEBUG, "request: %s\n", s->buffer);
 
+    if (strlen(headers) + 1 == sizeof(headers) ||
+        ret >= sizeof(s->buffer)) {
+        av_log(h, AV_LOG_ERROR, "overlong headers\n");
+        err = AVERROR(EINVAL);
+        goto done;
+    }
+
+
     if ((err = ffurl_write(s->hd, s->buffer, strlen(s->buffer))) < 0)
         goto done;