On Sat, May 11, 2013 at 01:28:26PM +0300, Daniel Shahaf wrote: > Stefan Sperling wrote on Tue, Mar 05, 2013 at 14:24:32 +0100: > > If the temp file is within a system-wide temp dir, and we end up expanding > > When exactly is that going to happen? You call svn_stream_open_unique() > with dirpath != NULL, so the tempdir it will use is ./ (neither /tmp or > .svn/tmp/).
It's not going to live in the system tempdir in the case under discussion. My point was that I would not like the code to be reused in a context where the file could end up in /tmp (where dirpath is a system tempdir). We could add a comment to explain the problem -- then I would be fine with any approach that works. > > in which case this doesn't really matter. If the working copy contains > > secrets it had better be within a directory that untrusted users cannot > > access. But I think the code should at least set a good example. > > > > So, can someone explain to me what the "other" race condition is? The > current code is racy because DST_TMP has (0777 &~ umask) perms while it > is being written to. DST_TMP has more restrictive permissions than that. It is created with the mkstemp() function under the hood. So we can assume that the perms of DST_TMP are configured such that only the user running svn can access the file. So we always start off with something like 600 on DST_TMP. And we need to preserve the permission bits of the existing DST (whatever they are) when renaming DST_TMP on top of it.
