>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
>Closed-Date:
>Last-Modified:
>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:
>Unformatted: