Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   Debian Development (http://www.linux-archive.org/debian-development/)
-   -   Bug#614601: ITP: libsafewrite -- Simple functions for performing safe atomic replacement of files (http://www.linux-archive.org/debian-development/492714-bug-614601-itp-libsafewrite-simple-functions-performing-safe-atomic-replacement-files.html)

Shachar Shemesh 02-22-2011 01:41 PM

Bug#614601: ITP: libsafewrite -- Simple functions for performing safe atomic replacement of files
 
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.



--
To UNSUBSCRIBE, email to debian-devel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20110222144115.9844.58744.reportbug@dellosun.offic e.lingnu.com">http://lists.debian.org/20110222144115.9844.58744.reportbug@dellosun.offic e.lingnu.com

Ben Hutchings 02-22-2011 04:54 PM

Bug#614601: ITP: libsafewrite -- Simple functions for performing safe atomic replacement of files
 
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


--
To UNSUBSCRIBE, email to debian-devel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20110222175450.GK28659@decadent.org.uk">http://lists.debian.org/20110222175450.GK28659@decadent.org.uk

Shachar Shemesh 02-23-2011 02:51 AM

Bug#614601: ITP: libsafewrite -- Simple functions for performing safe atomic replacement of files
 
On 22/02/11 19:54, Ben Hutchings wrote:



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?


Feel free not to use it/file bugs against it. Giving feedback over the
upstream trustworthiness is not the purpose of ITP bugs, and I have been
warned by the list masters that discussing a specific package's upstream
bugs on Debian-devel is off topic.

I quickly reviewed the code and found:


Did you read the accompanying manual pages first?

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


Thank you for your input. I'll fix it.

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

The matter has been discussed before. If you have a specific scenario
where this will cause a security flaw, please feel free to file a bug or
contact me directly. Pending that happening, my analysis is that there
is no security flaw in that case.

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


Interesting point. I'll have to think about it.

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

Frankly, I was not aware of this. I could not find it documented in the
man pages. In any case, this is no regression from the non-safe_open
case, as these would get cleared on write either way. If this is a Linux
only feature, I'm actually inclined to leave the code in (which is why I
needed the manual pages).

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

But consistent with what the man page says about it. The alternative is
to not allow the user to retry saving the file's content, which I don't
see as preferable.


Thank you for your feedback.
Shachar

--
Shachar Shemesh
Lingnu Open Source Consulting Ltd.
http://www.lingnu.com


--
To UNSUBSCRIBE, email to debian-devel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 4D648429.5010108@debian.org">http://lists.debian.org/4D648429.5010108@debian.org


All times are GMT. The time now is 05:47 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.