Bug#614601: ITP: libsafewrite -- Simple functions for performing safe atomic replacement of files

[Ben Hutchings <ben@decadent.org.uk>]

On Tue, Feb 22, 2011 at 04:41:15PM +0200, Shachar Shemesh wrote:
> Package: wnpp
> Severity: wishlist
> Owner: Shachar Shemesh <shachar@debian.org>
> 
> 
> * Package name    : libsafewrite
>   Version         : 1.00
>   Upstream Author : Shachar Shemesh <shachar@lingnu.com>
> * URL             : http://www.lingnu.com/opensource/safewrite.html
> * License         : MIT
>   Programming Lang: C
>   Description     : Simple functions for performing safe atomic file updates
> 
> Safewrite is a library for simple, almost drop-in replacement to the usual
> open and close calls. Using safewrite, however, guarantees that the files be
> updated in an atomic way - anyone trying to read the file is guaranteed to get
> a complete version, either the old or the new, but never a partially updated
> file.

Judging by what you consider 'small bugs' in
<https://github.com/Shachar/safewrite/commit/efafcd4260375a41257709c7eb5a8d6065366849>
why should anyone trust their important data to this library?

I quickly reviewed the code and found:

safe_open() might not return correct error codes, since the library
and system calls in its cleanup code may overwrite the original error
code.

It uses a fixed extension for the temporary file name, and unlinks
whatever was there before; this could be a security flaw.

It doesn't check for failure of fstat() (this is unlikely but possible,
e.g. when using a network filesystem).

Copying setuid and setgid bits to an empty file is pointless, since
they are cleared by write() (this is a good thing!).

safe_close() doesn't actually close the file or free the 'context' if
fsync() fails.  This is inconsistent with close().

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus