NetBSD Problem Report #19849
Received: (qmail 18864 invoked by uid 605); 14 Jan 2003 10:35:46 -0000
Message-Id: <20030114113543.6dc26a60.christianbiere@gmx.de>
Date: Tue, 14 Jan 2003 11:35:43 +0100
From: Christian Biere <christianbiere@gmx.de>
Sender: gnats-bugs-owner@netbsd.org
To: gnats-bugs@gnats.netbsd.org
Subject: Race conditions in compress
>Number: 19849
>Category: bin
>Synopsis: Race conditions in compress
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: bin-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Tue Jan 14 10:36:00 +0000 2003
>Closed-Date:
>Last-Modified: Thu Jan 23 19:08:01 +0000 2003
>Originator: Christian Biere
>Release: NetBSD 1.6K
>Organization:
>Environment:
>Description:
By reading PR bin/19722 I discovered
/usr/src/usr.bin/compress/compress.c:
In compress:
if (!isstdout) {
exists = !stat(out, &sb);
if (!force && exists && S_ISREG(sb.st_mode) &&
!permission(out)) return;
oreg = !exists || S_ISREG(sb.st_mode);
} else
oreg = 0;
IMHO, it's better to open the file and use ENOENT == errno to decide
whether the file exists or not.
if ((ifp = fopen(in, "r")) == NULL) {
cwarn("%s", in);
return;
}
IMO here's a real race condition:
if (!isstdin) {
if (stat(in, &isb)) { /* DON'T FSTAT! */
cwarn("%s", in);
goto err;
}
Why "DON'T FSTAT"? This would be the only way to prevent a race
condition. What's the reason for not using "fstat(fileno(ifp), &isb)"?
If you don't, there's no guarantee that you stat() the same file you've
opened with fopen() before.
Another example for the same kind of race condition:
if (fclose(ofp)) {
cwarn("%s", out);
goto err;
}
ofp = NULL;
if (isreg && oreg) {
if (stat(out, &sb)) {
cwarn("%s", out);
goto err;
[...]
Here, compress might modify the mode for the wrong file as it's only
identified by its name:
setfile(out, &isb);
In decompress:
Again, the file used with ofp is not guaranteed to be the same out
points to.
if (isreg && oreg) {
setfile(out, &sb);
>How-To-Repeat:
>Fix:
Giorgos Keramidas suggested to add a function zstat() to the zopen
interface. This would keep the code clean and wouldn't break the
interface.
>Release-Note:
>Audit-Trail:
From: Hubert Feyrer <hubert.feyrer@informatik.fh-regensburg.de>
To: Christian Biere <christianbiere@gmx.de>
Cc: gnats-bugs@gnats.netbsd.org
Subject: Re: bin/19849: Race conditions in compress
Date: Tue, 14 Jan 2003 12:38:45 +0100 (MET)
On Tue, 14 Jan 2003, Christian Biere wrote:
> if (stat(in, &isb)) { /* DON'T FSTAT! */
I think the idea here is to not (try to) compress symlinks.
- Hubert
--
Want to get a clue on IPv6 but don't know where to start? Try this:
* Basics -> http://www.onlamp.com/pub/a/onlamp/2001/05/24/ipv6_tutorial.html
* Setup -> http://www.onlamp.com/pub/a/onlamp/2001/06/01/ipv6_tutorial.html
Of course with your #1 IPv6 ready operating system -> http://www.NetBSD.org/
From: David Laight <david@l8s.co.uk>
To: netbsd-bugs@netbsd.org
Cc: gnats-bugs@gnats.netbsd.org
Subject: Re: bin/19849: Race conditions in compress
Date: Wed, 15 Jan 2003 17:47:10 +0000
> IMHO, it's better to open the file and use ENOENT == errno to decide
> whether the file exists or not.
>
> if ((ifp = fopen(in, "r")) == NULL) {
> cwarn("%s", in);
> return;
> }
You need to do the underlying open with O_NONBLOCK set just in
case a char special is specified (which might block - eg a tty
line waiting for carrier).
This starts getting hard if you want (need?) to keep portability
with non-unix systems.
OTOH there are an awful lot of these sort of race conditions
lurking, and fixing them will do no harm.
David
--
David Laight: david@l8s.co.uk
From: Christian Biere <christianbiere@gmx.de>
To: Hubert Feyrer <hubert.feyrer@informatik.fh-regensburg.de>
Cc: gnats-bugs@gnats.netbsd.org
Subject: Re: bin/19849: Race conditions in compress
Date: Thu, 23 Jan 2003 20:06:25 +0100
Hubert Feyrer <hubert.feyrer@informatik.fh-regensburg.de> wrote:
> I think the idea here is to not (try to) compress symlinks.
I was told, the following would be an appropriate method to prevent
accessing symlinks in this case:
Use fstat() to get the (effective) device id and inode, then use lstat()
to determine whether the file is a symlink, compare device id and inode
to check whether the file was exchanged between both calls.
Christian
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.