FreeBSD Port: mpack-1.6

[Paul Schmehl]

--On Wednesday, March 01, 2006 15:35:40 -0500 Kris Kennaway 
<kris at obsecurity.org> wrote:

> On Wed, Mar 01, 2006 at 02:08:22PM -0600, Paul Schmehl wrote:
>
>> So the chances of overwriting a file with the same random char set is
>> close  to nil.
>
> Close to nil != nil.
>
> I haven't read the code here, but secure handling of temp files
> requires care.  The code should just use mkstemp() though.
>
That's included in the patches.  That was one that Sergey added after I 
submitted the update.  I have now included it in the (new) patch.

--- unixpk.c.orig       Wed Mar  1 09:28:13 2006
+++ unixpk.c    Wed Mar  1 09:31:09 2006
@@ -164,10 +164,10 @@
            strcpy(fnamebuf, getenv("TMPDIR"));
        }
        else {
-           strcpy(fnamebuf, "/usr/tmp");
+           strcpy(fnamebuf, "/tmp");
        }
        strcat(fnamebuf, "/mpackXXXXXX");
-       mktemp(fnamebuf);
+        close(mkstemp(fnamebuf));
        outfname = strsave(fnamebuf);
     }

But I'm still wondering why you would use O_EXCL as a file descriptor on a 
new file that you're creating, unless you create and write in one operation.

>From man (2) open

O_EXCL          error if create and file exists

If O_EXCL is set with O_CREAT and the
     file already exists, open() returns an error.  This may be used to 
imple-
     ment a simple exclusive access locking mechanism.  If O_EXCL is set and
     the last component of the pathname is a symbolic link, open() will fail
     even if the symbolic link points to a non-existent name.

That's fine if you only use it when you first create the file, but when you 
then try to open the file for writing later, this attribute causes the 
"File already exists" error, because the file really does exist.  So you 
create a file that you can then not write to, which is what's happening 
here.

Paul Schmehl (pauls at utdallas.edu)
Adjunct Information Security Officer
University of Texas at Dallas
AVIEN Founding Member
http://www.utdallas.edu/ir/security/