Red Hat Bugzilla – Bug 832532
CVE-2012-2737 accountsservice: local file disclosure flaw
Last modified: 2012-08-16 11:35:24 EDT
Florian Weimer found a local file disclosure flaw in accountsservice, an account management system using D-Bus for querying and manipulating user accounts. The implementation of the SetIconFile method of the org.freedesktop.Accounts.User D-Bus interface can disclose arbitrary files due to a race condition in user_change_icon_file_authorized_cb() in /usr/libexec/accounts-daemon. When this function calls get_caller_uid(), it uses PolicyKit to obtain the UID of the requesting process from /proc. At the time the UID is fetched, it may not match the original UID making the D-Bus request if the process has executed an SUID binary.
So this code got added here:
Be more careful when copying the icon file
Don't read the file in the root process, instead fork, become the calling user, then read the file and pipe it back to the parent process. This protects against callers passing e.g. "/etc/shadow" as filename.
Matthias and I talked through this, this morning.
I think ideally we'd have the client open the file and pass the fd to the accounts daemon, but failing that, we need to check the peer credentials collected from the dbus-daemon at the time the client authorized with the daemon. DBus provides that information with the GetConnectionUnixUser method.
Is there any sort of embargo being organized?
Created attachment 593003 [details]
util: CVE-2012-2737: Validate SetIconFile caller over bus
The AccountsService SetIconFile call associates an icon
with a user.
This method allows users to have icons at the login screen,
that don't necessarily originate in globally readable/always
available locations. This is accomplished by copying the
originating icon to the local disk in /var.
Since AccountsService runs with escalated privileges, the
implemention of the SetIconFile method queres the callers
uid, forks(), assumes that uid and performs the copy as if
it were the user.
Unfortunately, the UID look up peformed is done "just in time"
instead of looking at peer credentials from the time the call
was initiated. This is a race condition that means a caller
could invoke the method call, quickly exec a setuid binary, and
then cause the copy to be performed as the uid of the setuid
This commit changes the uid look up logic, to query the system
bus daemon for peer credentials it cached from the caller at the
time of the call.
Created attachment 593044 [details]
Drop all uses of polkit_unix_process_get_uid
I discussed attachment 593003 [details] with davidz and mitr on IRC. They both agree it's a viable approach.
mitr did some additional auditing and discovered a few other callers of polkit_unix_process_get_uid(), so this patchset takes care of them as well.
The patches look fine to me
Florian, can you review the patch as well?
For the embargo, there is none set at the moment. As this is just in Fedora (do any other distributions use this?), once we get the patch vetted we can send it along to linux-distros with a short embargo, or skip the embargo altogether and notify oss-security instead.
(In reply to comment #8)
> Florian, can you review the patch as well?
The patch looks reasonable to me. Dbus uses credential passing to obtain the UID, so this should be fine (but I haven't looked at Dbus in detail).
> For the embargo, there is none set at the moment. As this is just in Fedora
> (do any other distributions use this?)
Debian packages acountsservice, but not in the stable distribution. Canonical might have released this version already.
accountsservice is a requirement of GNOME and is probably used by most recent distributions that ship GNOME.
Oh, ok. So if I send the details and the patch to linux-distros today, would an unembargo of Monday be acceptable?
CRD has passed, so making this bug public now.
Created accountsservice tracking bugs for this issue
Affects: fedora-all [bug 836284]
Patches went upstream now:
This is corrected via accountsservice-0.6.21-2.fc17 in Fedora 17.