[vlc-devel] [PATCH v2 13/13] vorepository: use vlc_MakeTmpFile (thus mkstemp) instead of tempnam

[Lyndon Brown]

On Tue, 2020-10-06 at 18:10 +0300, Rémi Denis-Courmont wrote:
> Le tiistaina 6. lokakuuta 2020, 10.20.07 EEST Steve Lhomme a écrit :
> > On 2020-10-06 8:02, Lyndon Brown wrote:
> > > From: Lyndon Brown <jnqnfe at gmail.com>
> > > Date: Tue, 6 Oct 2020 02:55:30 +0100
> > > Subject: vorepository: use vlc_MakeTmpFile (thus mkstemp) instead
> > > of
> > > tempnam
> > > 
> > > resolves warning about insecure function
> > > 
> > > diff --git a/modules/misc/addons/vorepository.c
> > > b/modules/misc/addons/vorepository.c index 06f02ce239..72b751c775
> > > 100644
> > > --- a/modules/misc/addons/vorepository.c
> > > +++ b/modules/misc/addons/vorepository.c
> > > @@ -400,20 +400,10 @@ static int Retrieve( addons_finder_t
> > > *p_finder,
> > > addon_entry_t *p_entry )> 
> > >           FREENULL( p_finder->p_sys->psz_tempfile );
> > >       
> > >       }
> > > 
> > > -    p_finder->p_sys->psz_tempfile = tempnam( NULL, "vlp" );
> > > -    if ( !p_finder->p_sys->psz_tempfile )
> > > -    {
> > > -        msg_Err( p_finder, "Can't create temp storage file" );
> > > -        vlc_stream_Delete( p_stream );
> > > -        return VLC_EGENERIC;
> > > -    }
> > > -
> > > -    int fd = vlc_open( p_finder->p_sys->psz_tempfile,
> > > -                       O_WRONLY | O_CREAT | O_EXCL, 0600 );
> > > +    int fd = vlc_MakeTmpFile(&p_finder->p_sys->psz_tempfile,
> > > PACKAGE_NAME"-vlp.XXXXXX", NULL);
> > There's a slight difference with the original code. vlc_mkstemp()
> > uses
> > O_RDWR while this code only uses O_WRONLY. It's worth mentioning it
> > in
> > the commit log.
> 
> This could just as well use vlc_mkstemp() and not have that problem. 

But the O_RDWR difference itself comes from vlc_mkstemp() used by the
new helper, not the helper itself...

> But 
> really, I don't see why this code needs to have a temporary file name
> at all. 
> In this particular case, tempnam() looks like just a ugly lazy hack:
> the file 
> name is never given to an external process that would somehow need a
> file name.

So a switch to use of vlc_memfd?

All I'm doing here is micro-targetting the cause of the build warning
about use of tempnam(). I'm not familiar enough with the overall code
of the module at this time to do much more than that, and it's not an
ideal time to get much more familiar with it. If you or someone else
would like to step in to implement a bigger change instead, I'm fine
with that.