[darcs-users] darcs patch: Create temporary files in temporary dire... (and 1 more)

David Roundy droundy at darcs.net
Fri May 9 11:55:32 UTC 2008


On Thu, May 08, 2008 at 03:54:01PM -0700, Eric Kow wrote:
> Warning: I don't actually know if these work.
> They may even break something.
> 
> Thu May  8 23:40:42 BST 2008  Eric Kow <E.Y.Kow at brighton.ac.uk>
>   * Create temporary files in temporary directory.
>   
>   This is related to issue770 and possibly others: darcs sometimes attempts to
>   create temporary files in the current directory.  If the current directory is
>   one where the user does not have permission to write to, this may fail.  Using
>   a known temporary directory, i.e. specifically marked for that purpose, should
>   work better.

The problem with this change is that we use the current directory for
security reasons, since it's very hard to safely use the /tmp directory
when communicating with external programs.  e.g. every time we run darcs
push, darcs creates the patch bundle in a temporary file before applying
it.  If we create this file in /tmp, then a malicious user might be able to
cleverly create a substitute patch bundle with the same name, which would
subsequently be applied to our repository.  There is a lot of literature on
safely creating temp files, but having only perused that literature, it's
not clear to me that what we'd want to do is possible.  The safe thing is
to create the temp files in a directory that evil people don't have write
access to, e.g. our repository directory.  So that's what we do.

Changing this would require a code audit by someone who can convince me
(and any other list-readers) that he or she thoroughly understands the
possible race conditions and can explain how we avoid them in a way that I
can understand, and moreover can explain why we have good reason to believe
that new race conditions won't be accidentally introduced at some later
time.

One option would be to fall back to creating files in the /tmp dir only
when the current directory isn't writeable.  This ought to be safe, because
if a bad guy can make your repository unwriteable to you, he probably can
already corrupt your repository.

David

P.S. My understanding of files in /tmp is that any use of their filename is
a security hole.  i.e. you should only ever use the file descriptor that
was returned when the file was simultaneously created and opened.  I'm sure
this is a pessimistic simplification, but when it comes to security I think
the proper approach is to be pessimistic.

> Thu May  8 23:49:32 BST 2008  Eric Kow <E.Y.Kow at brighton.ac.uk>
>   * Windows: check the TEMP environment variable for the tempdir location.


More information about the darcs-users mailing list