diff mbox

[FFmpeg-devel,3/3] http: fix potentially dangerous whitespace skipping code

Message ID 20180308035357.18138-3-nfxjfg@googlemail.com
State Accepted
Commit b7d842c554b1fec051ca906f446f7311139c5725
Headers show

Commit Message

wm4 March 8, 2018, 3:53 a.m. UTC 
If the string consists entirely of whitespace, this could in theory
continue to write '\0' before the start of the memory allocation. In
practice, it didn't really happen: the generic HTTP header parsing code
already skips leading whitespaces, so the string is either empty, or
consists a non-whitespace. (The generic code and the cookie code
actually have different ideas about what bytes are whitespace: the
former uses av_isspace(), the latter uses WHITESPACES. Fortunately,
av_isspace() is a super set of the http.c specific WHITESPACES, so
there's probably no case where the above assumption could have been
broken.)
---
 libavformat/http.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Paul B Mahol March 16, 2018, 7:10 p.m. UTC | #1 
On 3/8/18, wm4 <nfxjfg@googlemail.com> wrote:
> If the string consists entirely of whitespace, this could in theory
> continue to write '\0' before the start of the memory allocation. In
> practice, it didn't really happen: the generic HTTP header parsing code
> already skips leading whitespaces, so the string is either empty, or
> consists a non-whitespace. (The generic code and the cookie code
> actually have different ideas about what bytes are whitespace: the
> former uses av_isspace(), the latter uses WHITESPACES. Fortunately,
> av_isspace() is a super set of the http.c specific WHITESPACES, so
> there's probably no case where the above assumption could have been
> broken.)
> ---
>  libavformat/http.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 59f90ac603..983034f083 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -760,6 +760,8 @@ static int parse_set_cookie(const char *set_cookie,
> AVDictionary **dict)
>      back = &cstr[strlen(cstr)-1];
>      while (strchr(WHITESPACES, *back)) {
>          *back='\0';
> +        if (back == cstr)
> +            break;
>          back--;
>      }
>
> --
> 2.16.1
>

LGTM
diff mbox

Patch

diff --git a/libavformat/http.c b/libavformat/http.c
index 59f90ac603..983034f083 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -760,6 +760,8 @@  static int parse_set_cookie(const char *set_cookie, AVDictionary **dict)
     back = &cstr[strlen(cstr)-1];
     while (strchr(WHITESPACES, *back)) {
         *back='\0';
+        if (back == cstr)
+            break;
         back--;
     }