wrong permission on xauthority file

Bug #685212 reported by Yves-Alexis Perez on 2010-12-04
258
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Light Display Manager
High
Unassigned

Bug Description

By default, the Xauthority file is created with mode 644, enabling any user to hijack another user X screen.

Attached patch seems to fix the problem for me.

CVE References

Yves-Alexis Perez (corsac) wrote :
Changed in lightdm:
status: New → Fix Committed
importance: Undecided → High
Changed in lightdm:
status: Fix Committed → Fix Released
Yves-Alexis Perez (corsac) wrote :

Hmmh, btw it might be cleaner to use G_FILE_CREATE_PRIVATE in g_file_replace() instead of changing the mode afterward, I just discovered that.

Robert Ancell (robert-ancell) wrote :

I've updated to do that

Yves-Alexis Perez (corsac) wrote :

Seems that the bug is back, since you replaced g_file_replace() by g_file_set_contents() which actually replaces atomically the whole file but then respects umask, which is not touched before doing that...

It has been reported in Debian as http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=721744

I'm not sure if it's possible to reopen a bug, so I'll report a new one if it's not.

Hi,

I've released lightdm 1.4.2 and 1.6.1 which have this fix backported.

On Thu, Sep 5, 2013 at 7:58 AM, Yves-Alexis Perez <email address hidden> wrote:

> Seems that the bug is back, since you replaced g_file_replace() by
> g_file_set_contents() which actually replaces atomically the whole file
> but then respects umask, which is not touched before doing that...
>
> It has been reported in Debian as http://bugs.debian.org/cgi-
> bin/bugreport.cgi?bug=721744
>
> I'm not sure if it's possible to reopen a bug, so I'll report a new one
> if it's not.
>
> ** Bug watch added: Debian Bug tracker #721744
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=721744
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/685212
>
> Title:
> wrong permission on xauthority file
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/lightdm/+bug/685212/+subscriptions
>

Yves-Alexis Perez (corsac) wrote :

I'm sorry but that doesn't fix the security bug at all. It doesn't fix the permissions if they are currently bad, and even if the .Xauthority file doesn't exist, fopen() won't do better than set_file_contents(). Actually checking for the return code of the function might have helped too, but anyway.

In any case, care should be taken to:

- fix the permissions of existing .Xauthority files in a secure way
- correctly set the umask (or use a function which does it) before writing a new file

Yves-Alexis Perez (corsac) wrote :

On mer., 2013-09-04 at 22:24 +0000, Robert Ancell wrote:
> I've released lightdm 1.4.2 and 1.6.1 which have this fix backported.

As said on the bug, the fix doesn't actually work.

Regards,
--
Yves-Alexis

Robert Ancell (robert-ancell) wrote :

Sorry, this was a reply to an email regarding the temporary files being left behind when updating Xauthority. Confirmed the bug is back - am working on a fix.

Robert Ancell (robert-ancell) wrote :

Should be fixed in 1.4.3, 1.6.2, 1.7.14

information type: Private Security → Public Security
information type: Public Security → Private Security
information type: Private Security → Public Security
Yves-Alexis Perez (corsac) wrote :

I guess it'd be best to also check for the current file permissions and change them if needed (or change them inconditionally if the file already exists).

I can try to cook a patch, I'm just not too sure how to be really clean wrt. links attacks etc, although at that point the file should be written as user so it's safer than if it was done as root.

To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.