Package: logrotate
Version: 3.7-5
Severity: important
Tags: security

In the code of logrotate there are multiple instances of code like the
following:

|    fstat(inFile, &sb);
|
|    if ((outFile = open(compressedName, O_RDWR | O_CREAT | O_TRUNC, 
sb.st_mode)) < 0) {
|        message(MESS_ERROR, "unable to open %s for compressed output\n",
|                compressedName);
|        close(inFile);
|        return 1;
|    }
|
|    if (fchown(outFile, sb.st_uid, sb.st_gid)) {
|        message(MESS_ERROR, "unable to change owner of output file %s\n",
|                compressedName);
|        close(inFile);
|        close(outFile);
|        return 1;
|    }

I'd argue that there is a race condition in there: If sb.st_mode
includes group permissions, this could (given the appropriate umask)
grant these permissions to the effective group of the creating process
in the moment of file creation.

As a user, I'd expect the "permission cloning" to happen in such a way
that it is guaranteed that access to the information contained in the log
file is always limited to what is specified by the log file's permissions.

In the default setup, this, of course, shouldn't be a problem, since
logrotate is run with an effective group of root, and any member of that
group will usually have access to the log files anyway. When logrotate
is used by normal users, though, this could be a security problem.

One possible solution would be to always create files with S_IRUSR|S_IWUSR
as the mode parameter to open and do a chmod to sb.st_mode after the
chown. I'm not sure, though, whether that would still leave problems
in some cases where an attacker could create the to-be-created file
beforehand and thus cause it to just be opened and truncated without
re-creation. The most-easily constructed scenario (directory writeable
by a user/group not allowed to read the log file) should be prevented by
the chown failing for a non-root logrotate process when trying to change
the owner of the destination file.

Unless anyone is sure that there are no problems left with this
solution (and for the sake of clarity maybe even then), I'd suggest
to instead unlink() the filename beforehand, then (re-)create it with
O_EXCL and mode of 0 (shouldn't cause any problems anymore, since
logrotate never will need to open it unless the later chmod has been
executed), then chown and then chmod the file to set the "cloned
permissions" (possibly obeying the current umask for backwards
compatibility, even though I do consider it rather counter-intuitive
that the umask is obeyed while cloning the permissions of one file onto
another).

The only case I can think of where this could cause problems is if
someone is currently creating the files before calling logrotate
in a place where logrotate would not be allowed to create them
itself. I somehow doubt, though, that anyone is doing that ...


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to