Date: Wed, 01 Mar 2006 14:08:22 -0600 From: Paul Schmehl <pauls@utdallas.edu> To: Sergey Matveychuk <sem@FreeBSD.org>, Boris Samorodov <bsam@ipt.ru> Cc: ports@FreeBSD.org Subject: Re: FreeBSD Port: mpack-1.6 Message-ID: <665EA8A520757A68F0485536@utd59514.utdallas.edu> In-Reply-To: <4405F6F0.9050703@FreeBSD.org> References: <44050D77.2030503@j2d.lam.net.au> <BCA5F50D2461133FF65B3BD8@utd59514.utdallas.edu> <84747890@srv.sem.ipt.ru> <4405F6F0.9050703@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--On Wednesday, March 01, 2006 22:33:04 +0300 Sergey Matveychuk
<sem@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@utdallas.edu)
Adjunct Information Security Officer
University of Texas at Dallas
AVIEN Founding Member
http://www.utdallas.edu/ir/security/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?665EA8A520757A68F0485536>