[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1452780 - /subversion/trunk/subversion/libsvn_subr/subst.c

From: Stefan Sperling <stsp_at_elego.de>
Date: Sat, 11 May 2013 13:31:52 +0200

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.
Received on 2013-05-11 13:32:43 CEST

This is an archived mail posted to the Subversion Dev mailing list.