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

[Thomas Guillem]

On Wed, Jul 13, 2016, at 16:55, Rémi Denis-Courmont wrote:
> 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.

Ah yes indeed. Normally I have a warning for that...

> 
> > 
> > > >      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/
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel