Thanks for the prompt response.
For the first issue, it's not a matter of reading a user's mail, but
causing mail deliveries to that user to overwrite other files owned by
that user. For example, if we use your example of victim "foo" and
attacker "bar", where "foo" has no mailbox, "bar" can create a
hardlink to another one of foo's files, such as /home/foo/.bashrc.
Subsequent mail delivery will append to this file, allowing an
attacker to append information to other users' files.
I'll walk through an example of exploitation of the second
vulnerability. Say attacker "bar" is delivering mail to user "foo".
At src/transports/appendfile.c, line 2008, Ulstat() is called to both
check the existence of a lockfile and whether or not it's a symlink or
hard link. Suppose there is no lockfile at this point. After the
Ulstat() call (but before subsequent calls), the attacker can create a
symlink to a non-existent file in a directory foo has permission to
write to, and the subsequent Uopen() call will create that file for
the attacker because of the O_CREAT flag. In other cases, the
attacker can instead create a symlink to an already existing file
owned by "foo", and the subsequent Uchmod will change the permissions
of the file pointed to by the symlink. In short, it's the space
between the Ulstat() call and either the Uopen() or Uchmod() calls
that causes the exploitable race condition. In practice, these sorts
of things are usually exploited by rapidly creating and deleting a
symlink in /tmp while repeatedly executing the vulnerable program.
I do understand why my proposed fix is unacceptable from a locking
standpoint, so of course feel free to disregard that part of the
patch. Hopefully this clarifies these issues - let me know if
anything's still fuzzy.
Thanks again,
Dan
On Tue, May 25, 2010 at 10:39 AM, Phil Pennock <pdp@???> wrote:
> On 2010-05-24 at 13:59 -0400, Dan Rosenberg wrote:
>> Hi. I've discovered two security vulnerabilities in Exim4:
>>
>> 1. When Exim is used with a world-writable mail directory with the
>> sticky-bit set, local users may create hard links to other non-root
>> users' files at the expected location of those users' mailboxes,
>> causing their files to be written to upon mail delivery. This could
>> be used to create denial-of-service conditions or potentially escalate
>> privileges to those of targeted users. This issue has been assigned
>> CVE-2010-2023.
>
> If user "foo" has no mailbox and attacker "bar" wants to have foo's mail
> delivered to bar, then "bar" creates a hardlink "foo" pointing to "bar".
>
> At this point, the file is owned by "bar" and the check_owner rule means
> that Exim will have refused to deliver because the file has the wrong
> uid. The administrator can choose to turn off "check_owner" on the
> appendfile transport, but it's on by default.
>
> So unless the mail admin has explicitly said that ownership is not an
> issue, I don't see the vulnerability. OTOH, I'm mildly jetlagged and
> possibly missing something.
>
> Do you have a test-case showing this vulnerability, please?
>
>> 2. When MBX locking is enabled, local users may exploit a race
>> condition to change permissions of other non-root users' files,
>> leading to denial-of-service conditions or potentially privilege
>> escalation, or to create new files owned by other users in
>> unauthorized locations. This issue has been assigned CVE-2010-2024.
>>
>> I've attached a patch for these issues. The first issue is resolved
>> by checking the link count of the file before writing - since we're
>> assuming a sticky bit, there's no potential for race conditions if the
>> file is successfully opened (because once a hard link to another
>> user's file is created, it cannot be removed). The second issue is
>> resolved by first attempting to open the file without O_CREAT. If it
>> succeeds, then we use fstat() to check that the file is not a symlink,
>> etc. If the open doesn't succeed, then we make another attempt using
>> O_CREAT | O_EXCL. If this fails, something fishy is going on.
>> Otherwise, we obtain a safe lock file.
>
> I've never touched the MBX code before and am jetlagged, but: this is
> locking code, it needs to protect against concurrent updates which can
> be triggered by mail delivery, so a "check and then creat/excl later"
> approach is going to get false positives -- it's not fishy to have mail
> be delivered while attempting to read mail.
>
> I do see the race condition, but am not seeing the exploit. Exim has
> dropped privileges to be running as the uid for which it's attempting to
> deliver mail. It is possible to create the second, /tmp-based,
> exclusive lock called for by MBX. Exim can either lock that file and
> deliver as normal, with the ownership of the /tmp-lock being at all
> times irrelevant, or Exim can't lock the file. If Exim can't lock the
> file, it will defer delivery and the mail will sit on the queue.
>
> There isn't even a race here: if the file already exists, it will be
> used if it's not a symlink and doesn't have increased link count. If it
> can't be exclusively locked then mail can't be delivered. This appears
> to be an MBX design weakness.
>
> There is some room for improvement here, using fstat() on the opened
> file descriptor to do more checks to have better diagnostics, but
> nothing I'm going to rush into without looking into what c-client is
> doing, as that's the model being followed.
>
> Generally, the impact is:
> * mail delivery can be deferred by inhibiting MBX's second lock
> and I'm not seeing the potential for privilege escalation. Can you
> point me at it please?
>
> Note that when delivery is deferred, the administrator can see the mails
> sitting in the queue and look at the per-message delivery log to see why
> delivery is being deferred. At which point, the lock file belonging to
> the attacking user points to the correct account and that account can be
> investigated for compromise and/or the human invited in for a Chat.
>
>
> Please note that irregardless of whatever I am or am not seeing right
> now, we really do appreciate people doing security reviews of Exim and
> contacting us with feedback -- thank you for this!
>
> Thanks,
> -Phil
>
