FreeBSD Port: mpack-1.6

[Paul Schmehl]

--On Wednesday, March 01, 2006 22:33:04 +0300 Sergey Matveychuk 
<sem at FreeBSD.org> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Boris Samorodov wrote:
>> Can't say for sure why that patch exists, but deletting
>> files/patch-unixpk_c should help. Cvs says "Yet another overflow
>> check, better temp file name & misc cleanups". Assume this comment is
>> for the whole commit.
>
> The patch fixes an insecure temp file handling. But I think the handling
> could be changed between versions and it's useless (even harm?) now.
>
I included it in my patch anyway.

I found the problem, but before I commit it, I want to make sure I'm not 
doing something stupid.

In unixos.c you will find this construct:

#ifdef O_EXCL
    fd=open(fname, O_RDWR|O_CREAT|O_EXCL, 0644);
#else
    fd=open(fname, O_RDWR|O_CREAT|O_TRUNC, 0644);
#endif

I don't understand this.  Why would you use O_EXCL here?  ISTM there's 
almost no chance of overwriting an existing file, because the filename 
created is mpackXXXXXX, where XXXXXX is generated this way:

93231 mpack    CALL  getpid
 93231 mpack    RET   getpid 93231/0x16c2f
 93231 mpack    CALL  gettimeofday(0xbfbfd368,0)
 93231 mpack    RET   gettimeofday 0
 93231 mpack    CALL  __sysctl(0xbfbfd368,0x2,0x804d5e0,0xbfbfd384,0,0)
 93231 mpack    RET   __sysctl 0
 93231 mpack    CALL  open(0x804f060,0xa02,0x1a4)
 93231 mpack    NAMI  "/tmp/mpackkZ1dQH"

So the chances of overwriting a file with the same random char set is close 
to nil.  But even *if* you overwrite the file, it's going to be one that 
you created previously using mpack, so what's the big deal?  That previous 
operation would have already concluded - successfully or unsuccessfully, 
right?

So I removed the ifdef statement and left just the file descriptor line:
fd=open(fname, O_RDWR|O_CREAT|O_TRUNC, 0644);

Now the program works as expected.  Can anyone tell me why you would want 
to use O_EXCL here?

If I haven't missed something really terrible, I'll commit the patches, and 
the port should be fixed (at least this problem.)

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