[vlc-devel] commit: Added a utf8_mkstemp implementation. (Laurent Aimar )

[Laurent Aimar]

On Tue, Nov 18, 2008, Rémi Denis-Courmont wrote:
> On Tuesday 18 November 2008 02:12:05 Laurent Aimar, you wrote:
> > > > +    uint64_t i_rand = mdate();
> > >
> > > This is (obviously) predictible pseudo-randomness, not immediately a
> > > problem, but...
> >
> >  I would have prefer to use a better seed but dunno what to use...
> 
> I think we have a random bytes function in the LibVLC API, as the RTSP server 
> needed it for Session IDs.
 Yes, I will use it.

> > > > +        int fd = utf8_open( template, O_CREAT | O_EXCL | O_RDWR, 0600
> > > > );
> > >
> > > ...we have an insecure file creation here. To avoid depending on
> > > O_NOFOLLOW, we should probably use mkstemp() on those platform which do
> > > have it.
> >
> >  I haven't though about links.
> >  I can use fstat and close it if it is a link. It would probably be safer
> > for a start.
> 
> It won't work. Symbolic links cannot be opened, so fstat() is impossible. The 
> path may lead to a symbolic link, which in turn may lead to a non-existent 
> file inside an existent directory. In that case, open() will succeed in 
> creating a file inspite of O_EXCL. If the symbolic link is in a directory 
> with different write permissions (typically /tmp) than what it points to, we 
> have a security bypass.
> 
> The only race-free solution is O_NOFOLLOW, which is non-standard.
 From my man page of open, I have

"O_EXCL When used with O_CREAT, if the file already exists it is an error and the open() will fail.  In  this  context,  a  symbolic  link  exists,
      regardless  of  where it points to."

 As the open will always fail if it is a symbolic link, I think it is perfectly
safe, do you agree ?

-- 
fenrir