[FFmpeg-devel] [PATCH 2/3] avfilter/graphparser: Don't set pointer to one beyond '\0' of string

[Nicolas George]

Andreas Rheinhardt (12020-08-23):
> This happened in parse_link_name() if there was a '[' without matching
> ']'. While this is not undefined behaviour (pointer arithmetic one
> beyond the end of an array works fine as long as there are no accesses),
> it is potentially dangerous. It currently isn't (all callers of
> parse_link_name() treat this as an error and don't access the string any
> more), but making sure that this will never cause trouble in the future
> seems nevertheless worthwhile.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavfilter/graphparser.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavfilter/graphparser.c b/libavfilter/graphparser.c
> index dfb94788e1..e96b20418e 100644
> --- a/libavfilter/graphparser.c
> +++ b/libavfilter/graphparser.c
> @@ -63,7 +63,7 @@ static char *parse_link_name(const char **buf, void *log_ctx)
>  
>      name = av_get_token(buf, "]");
>      if (!name)
> -        goto fail;
> +        return NULL;

This looks ok.

>  
>      if (!name[0]) {
>          av_log(log_ctx, AV_LOG_ERROR,
> @@ -71,12 +71,14 @@ static char *parse_link_name(const char **buf, void *log_ctx)
>          goto fail;
>      }
>  
> -    if (*(*buf)++ != ']') {
> +    if (**buf != ']') {
>          av_log(log_ctx, AV_LOG_ERROR,
>                 "Mismatched '[' found in the following: \"%s\".\n", start);
>      fail:
>          av_freep(&name);
> +        return NULL;
>      }
> +    (*buf)++;
>  
>      return name;
>  }

I would like it better if you took the opportunity to get rid of this
spaghetti goto. A fail label in an if that uses the side effect of
av_freep() to make sure the final return will return NULL? Brr, that's
bound to come back and bite us at some time.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200823/42e40587/attachment.sig>