[vlc-devel] [PATCH 08/12] input: subtitles: Remove use of VLA

[Rémi Denis-Courmont]

Le mardi 8 décembre 2020, 16:19:12 EET Hugo Beauzée-Luyssen a écrit :
> ---
>  src/input/subtitles.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/src/input/subtitles.c b/src/input/subtitles.c
> index 25ebb52087..6598aaa0fc 100644
> --- a/src/input/subtitles.c
> +++ b/src/input/subtitles.c
> @@ -283,9 +283,22 @@ int subtitles_Detect( input_thread_t *p_this, char
> *psz_path, const char *psz_na if( psz_name[0] == '.' || !subtitles_Filter(
> psz_name ) ) continue;
> 
> -            char tmp_fname_noext[strlen( psz_name ) + 1];
> -            char tmp_fname_trim[strlen( psz_name ) + 1];
> -            char tmp_fname_ext[strlen( psz_name ) + 1];
> +            char *tmp_fname_noext = malloc(strlen( psz_name ) + 1);
> +            if (!tmp_fname_noext)
> +                continue;
> +            char *tmp_fname_trim = malloc(strlen( psz_name ) + 1);
> +            if (!tmp_fname_trim)
> +            {
> +                free(tmp_fname_noext);
> +                continue;
> +            }
> +            char *tmp_fname_ext = malloc(strlen( psz_name ) + 1);
> +            if (!tmp_fname_ext)
> +            {
> +                free(tmp_fname_noext);
> +                free(tmp_fname_trim);
> +                continue;
> +            }
>              const char *tmp;
>              int i_prio = 0;
> 

I don't know if filenames are guaranteed to be small enough to fit. But since it 
is now forbidden to post or expose new vulnerabilities, I assume that they are 
small enough to fit.

That being the case, this change is completely unnecessary. Eror handling is 
the single most common cause of bugs in VLC. And this is adding error cases 
that are all but guaranteed to cause leaks or invalid frees when the function 
gets revectored.

And while there is no performance concern here, I would think that using the 
heap could be done in a more legible and compact manner.

-- 
Rémi Denis-Courmont