[vlc-devel] [PATCH] Stop realm variable in modules/access/http.c from being freed too early.

[Alexandre Janniaux]

Hi,

Thanks for the patch, but I'm not sure of what issue it is
fixing. Do you have asan reports of use after free?

It seems to be init in Connect and is not freed before the
end of this code as far as I can read, so if you have random
data it probably comes from a different part of the code.

Also you can use strdup instead of malloc(strlen) + strcpy

Regards,
--
Alexandre Janniaux
Videolabs

On Fri, May 08, 2020 at 01:27:05AM +1000, Aaron Wyatt via vlc-devel wrote:
> Date: Fri, 8 May 2020 01:27:05 +1000
> From: Aaron Wyatt <github at psi-borg.org>
> To: vlc-devel at videolan.org
> Subject: [PATCH] Stop realm variable in modules/access/http.c from being
>  freed too early.
> User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0)
>  Gecko/20100101 Thunderbird/68.7.0
>
> Fix to stop the realm variable in modules/access/http.c from being freed
> before credentials are stored using the keystore module. (Prevents keystore
> modules from creating multiple entries with keys based on junk data.)
>
> Aaron
>
> ---
>  modules/access/http.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/modules/access/http.c b/modules/access/http.c
> index 4384e2b0a3..b4645c0f4e 100644
> --- a/modules/access/http.c
> +++ b/modules/access/http.c
> @@ -142,6 +142,7 @@ static int Open( vlc_object_t *p_this )
>      char *psz;
>      int ret = VLC_EGENERIC;
>      vlc_credential credential;
> +    char *psz_realm;
>       access_sys_t *p_sys = vlc_obj_malloc( p_this, sizeof(*p_sys) );
>      if( unlikely(p_sys == NULL) )
> @@ -166,6 +167,7 @@ static int Open( vlc_object_t *p_this )
>      p_sys->offset = 0;
>      p_sys->size = 0;
>      p_access->p_sys = p_sys;
> +    psz_realm = NULL;
>       if( vlc_UrlParse( &p_sys->url, psz_url ) || p_sys->url.psz_host ==
> NULL )
>      {
> @@ -298,7 +300,10 @@ connect:
>          msg_Dbg( p_access, "authentication failed for realm %s",
>                   p_sys->auth.psz_realm );
>  -        credential.psz_realm = p_sys->auth.psz_realm;
> +        free( psz_realm );
> +        psz_realm = malloc( strlen( p_sys->auth.psz_realm ) + 1 );
> +        strcpy( psz_realm, p_sys->auth.psz_realm );
> +        credential.psz_realm = psz_realm;
>          credential.psz_authtype = p_sys->auth.psz_nonce  ? "Digest" :
> "Basic";
>           if( vlc_credential_get( &credential, p_access, NULL, NULL,
> @@ -339,6 +344,8 @@ connect:
>      p_access->pf_control = Control;
>      p_access->pf_seek = Seek;
>  +    free( psz_realm );
> +    psz_realm = NULL;
>      vlc_credential_clean( &credential );
>       return VLC_SUCCESS;
> @@ -347,6 +354,8 @@ disconnect:
>      Disconnect( p_access );
>   error:
> +    free( psz_realm );
> +    psz_realm = NULL;
>      vlc_credential_clean( &credential );
>      vlc_UrlClean( &p_sys->url );
>      if( p_sys->b_proxy )
> --
> 2.24.2 (Apple Git-127)
>

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel