From owner-freebsd-ports@FreeBSD.ORG Wed Mar 1 20:08:23 2006 Return-Path: X-Original-To: ports@FreeBSD.org Delivered-To: freebsd-ports@FreeBSD.ORG Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 274DA16A420; Wed, 1 Mar 2006 20:08:23 +0000 (GMT) (envelope-from pauls@utdallas.edu) Received: from smtp1.utdallas.edu (smtp1.utdallas.edu [129.110.10.12]) by mx1.FreeBSD.org (Postfix) with ESMTP id D65CB43D48; Wed, 1 Mar 2006 20:08:22 +0000 (GMT) (envelope-from pauls@utdallas.edu) Received: from utd59514.utdallas.edu (utd59514.utdallas.edu [129.110.3.28]) by smtp1.utdallas.edu (Postfix) with ESMTP id 8FB42388EF4; Wed, 1 Mar 2006 14:08:22 -0600 (CST) Date: Wed, 01 Mar 2006 14:08:22 -0600 From: Paul Schmehl To: Sergey Matveychuk , Boris Samorodov Message-ID: <665EA8A520757A68F0485536@utd59514.utdallas.edu> In-Reply-To: <4405F6F0.9050703@FreeBSD.org> References: <44050D77.2030503@j2d.lam.net.au> <84747890@srv.sem.ipt.ru> <4405F6F0.9050703@FreeBSD.org> X-Mailer: Mulberry/3.1.6 (Linux/x86) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline Cc: ports@FreeBSD.org Subject: Re: FreeBSD Port: mpack-1.6 X-BeenThere: freebsd-ports@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting software to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Mar 2006 20:08:23 -0000 --On Wednesday, March 01, 2006 22:33:04 +0300 Sergey Matveychuk 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/