Subject: bin/19849: Race conditions in compress
To: None <>
From: Christian Biere <>
List: netbsd-bugs
Date: 01/14/2003 11:35:43
>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 02:36:00 PST 2003
>Originator:     Christian Biere
>Release:        NetBSD 1.6K


By reading PR bin/19722 I discovered


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);

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);



Giorgos Keramidas suggested to add a function zstat() to the zopen
interface. This would keep the code clean and wouldn't break the