Hi, On 20/09/16 18:09, salsaman wrote: > As I mentioned already, the location of this directory is selected by > the user the first time that LiVES is run. > There is nothing forcing it to be ~/livestmp.
That's fine, although I don't think it should be the default. > The directory being world writeable is a separate bug which will be > addressed there. > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=798043 Ok. >>> Regarding the other files: >>> >>> /tmp/lives-symlinks is only used in the Dynebolic bootable disk. >>> Effectively this can be ignored, since it is not used in normal operation. >> >> Why does the code still exist then? Can you confirm that it can never >> ever be executed? >> >> I have not investigated much, but if it can be executed then it could >> quite easily be used to allow editing of any file in a user's home >> directory. [...] > I rechecked and you may be correct in this case. I started reworking > this code now. /tmp will no longer used, instead all the symlinks will > be created in the individual clip directories. In addition, the > directory was set with chmod 777, now it will be created with mode 700. That sounds good. >> For example, simply grepping the lives source reveals: >> lives-plugins/plugins/playback/video/oggstream.c: >> dummyvar=system("smogrify get_tempdir oggstream"); >> >> This allows any user to truncate any file owned by the lives user by >> simply creating a symlink, and waiting for smogrify to be run. >> ln -s $IMPORTANT_FILE /tmp/.smogrify.oggstream > > Thanks for pointing that out. > The only other place where I know /tmp is used is in > lives-plugins/marcos-encoders. For example in lives_mkv_encoder we have: > temp_dir = tempfile.mkdtemp('', '.lives-', '/tmp/') > > However the description of (Python) mkdtemp suggests that this should be > safe: > https://docs.python.org/2/library/tempfile.html > > "Creates a temporary directory in the most secure manner possible. There > are no race conditions in the directory’s creation. The directory is > readable, writable, and searchable only by the creating user ID." Yep mkdtemp should be secure so there's no problem here. > So to summarise - issues to be addressed: > > - remove old unneeded code with rm /tmp/.smogval > - do not create symlinks in /tmp, instead create them in the clip > directories > - in the case where values are written to /tmp for external scripts > (smogrify get_tempdir), check that the file exists and is owned by the user Thinking about this some more, there is a slight race condition here if the user deletes the file after the checks, but before it's written. I think the best fix would break the smogrify API unfortunately. One alternative is to use to use open(2)'s O_CREATE | O_EXCL flags, but this will only work if the file does not exist beforehand. > Is there anything else I have missed ? I think that's everything I found. Thanks, James
signature.asc
Description: OpenPGP digital signature