On Wednesday 07 October 2009 06:45:09 Felix Wolfsteller wrote: > On Tuesday 06 October 2009 23:52:25 Tim Brown wrote: > > Whilst doing some janitor work on the OpenVAS code base I spotted a > > potential race condition. > > > > openvas-scanner/openvassd/utils.c contains a function temp_file_name() > > which contains a loop that recusively constructs random filenames under > > OPENVASSD_STATEDIR/tmp and then checks whether they exist by attempting > > to open them with O_RDONLY. When open returns a file descriptor >= 0, > > the filename is returned. > > > > This function is called from openvas-scanner/openvassd/ntp_11.c by the > > ntp_11_recv_file() function. This function opens the filename returned > > with O_CREAT|O_WRONLY|O_TRUNC and then writes to it. > > > > There exists a time of check, time of use race condition which might be > > exploited to overwrite arbitrary files. > > > > Thoughts before I start to clean it up? > > I would say go ahead. > > > ATTACHED_FILE still seems to be supported by OTP 1.0, no? > > Yes, its needed for credentials assignment (comes in form of a file) and > for nvts that require a "File" preference.
Hmm, this is actually a lot more bothersome that I first thought....the lifetime of the files seems to be quite long. After generating the filename it writes to it and then does daft things like: files_add_translation (globals, origname, localname); Using mkstemp alone won't be enough although it did solve the time of check time of write which inititially caught my eye :(. Seems we need to cache the filename we generate, the device ID and the inode and check this on each subsequent read but it seems like a real nasty solution. Either that or hold the contents in memory and bin the whole write to disk element entirely. Suggestions? Tim -- Tim Brown <mailto:t...@openvas.org> <http://www.openvas.org/> _______________________________________________ Openvas-devel mailing list Openvas-devel@wald.intevation.org http://lists.wald.intevation.org/mailman/listinfo/openvas-devel
