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

2018-03-16 Thread Paul B Mahol
On 3/8/18, wm4  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 = [strlen(cstr)-1];
>  while (strchr(WHITESPACES, *back)) {
>  *back='\0';
> +if (back == cstr)
> +break;
>  back--;
>  }
>
> --
> 2.16.1
>

LGTM
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2018-03-07 Thread wm4
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 = [strlen(cstr)-1];
 while (strchr(WHITESPACES, *back)) {
 *back='\0';
+if (back == cstr)
+break;
 back--;
 }
 
-- 
2.16.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel