[vlc-devel] [PATCH] gnutls: fix stored pubkey on Android

[Rémi Denis-Courmont]

Le keskiviikkona 13. heinäkuuta 2016, 16.51.04 EEST Thomas Guillem a écrit :
> On Wed, Jul 13, 2016, at 16:03, Rémi Denis-Courmont wrote:
> > This looks like a problem between Android and GnuTLS, and hacking around
> > it in
> > LibVLC looks questionable.
> 
> I'll patch gnutls in contribs then.
> 
> > Also...
> > 
> > Le keskiviikkona 13. heinäkuuta 2016, 15.45.30 EEST Thomas Guillem a
> > 
> > écrit :
> > > ---
> > > 
> > >  modules/misc/gnutls.c | 18 ++++++++++++++++--
> > >  1 file changed, 16 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/modules/misc/gnutls.c b/modules/misc/gnutls.c
> > > index 1b9c315..e3a4eb2 100644
> > > --- a/modules/misc/gnutls.c
> > > +++ b/modules/misc/gnutls.c
> > > @@ -460,8 +460,21 @@ static int gnutls_ClientHandshake(vlc_tls_creds_t
> > > *creds, vlc_tls_t *tls, goto error;
> > > 
> > >      }
> > > 
> > > +    char *db_name = NULL;
> > > +#ifdef __ANDROID__
> > > +    /* gnutls fail to save the known_hosts file on Android since there
> > > is
> > > no +     * valid HOME directory */
> > > +    char *config = config_GetUserDir(VLC_CONFIG_DIR);
> > > +    if (config != NULL)
> > > +    {
> > > +        if (asprintf(&db_name, "%s/gnutls_known_hosts", config) == -1)
> > > +            db_name = NULL;
> > > +        free(config);
> > > +    }
> > > +#endif
> > > +
> > > 
> > >      msg_Dbg(creds, "%u certificate(s) in the list", count);
> > > 
> > > -    val = gnutls_verify_stored_pubkey (NULL, NULL, host, service,
> > > +    val = gnutls_verify_stored_pubkey (db_name, NULL, host, service,
> > > 
> > >                                         GNUTLS_CRT_X509, datum, 0);
> > >      
> > >      const char *msg;
> > >      switch (val)
> > > 
> > > @@ -522,7 +535,7 @@ static int gnutls_ClientHandshake(vlc_tls_creds_t
> > > *creds, vlc_tls_t *tls, time (&expiry);
> > > 
> > >              expiry += 24 * 60 * 60;
> > >          
> > >          case 2:
> > > -            val = gnutls_store_pubkey (NULL, NULL, host, service,
> > > +            val = gnutls_store_pubkey (db_name, NULL, host, service,
> > > 
> > >                                         GNUTLS_CRT_X509, datum, expiry,
> > >                                         0);
> > >              
> > >              if (val)
> > >              
> > >                  msg_Err(creds, "cannot store X.509 certificate: %s",
> > > 
> > > @@ -536,6 +549,7 @@ static int gnutls_ClientHandshake(vlc_tls_creds_t
> > > 
> > > *creds, vlc_tls_t *tls, error:
> > >      if (alp != NULL)
> > >      
> > >          free(*alp);
> > > 
> > > +    free(db_name);
> > 
> > Looks like undefined behaviour to me. I expect compiler warning even.
> 
> There is a leak in case of success, but I don't see any undefined
> behaviour (no warnings).

AFAICT, db_name is unspecified in case of jump past initialization and thus 
free(db_name) is undefined.

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


-- 
Rémi Denis-Courmont
http://www.remlab.net/