Ubuntu
lightdm package

lightdm should not rename() .xsession-errors, as it may be a symlink

Bug #1108518 reported by Alec Warner
24
This bug affects 5 people
Affects Status Importance Assigned to Milestone
lightdm (Ubuntu)
Confirmed
Low
Unassigned

Bug Description

http://bazaar.launchpad.net/~ubuntu-desktop/lightdm/ubuntu/view/head:/src/session-child.c#L509

Lightdm calls rename() on the xsession-errors file. In our deployment many of our users have this file as a symlink. Their homedirectory is on NFS and it will quickly fill their quota with bogus messages.

You should lstat() the file first. If it is a symlink, you can make a new file at the destination and update the link. I can supply a patch if you wish.

antarus@goats5:/etc/X11/Xsession.d$ apt-cache policy lightdm
lightdm:
  Installed: 1.2.1-0ubuntu1.1
  Candidate: 1.2.1-0ubuntu1.1
  Version table:

DISTRIB_CODENAME=precise
DISTRIB_DESCRIPTION="Ubuntu 12.04.1 LTS"
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=12.04

-A

Revision history for this message
Sebastien Bacher (seb128) wrote :

Thank you for your bug report, the people working on lightdm are pretty busy with other projects as well, having a patch up for review would be very welcome if you want to work on one ;-)

Note that the best to submit one for review is through a merge request, you can do one following those steps:

bzr branch lp:lightdm
cd lightdm
<do your changes>
bzr commit
bzr push lp:~<yourlpid>/lightdm/xsession-errors-symlink-fix
bzr lp-open

then select "submit for review" on the page and file the description

Thanks in advance!

Changed in lightdm (Ubuntu):
importance: Undecided → Low
status: New → Confirmed
Revision history for this message
Robert Ancell (robert-ancell) wrote :

Is there an atomic way of doing this? Otherwise the file could be replaced with a symlink between the lstat and the copy.

Revision history for this message
Alec Warner (antarus) wrote :

My point isn't so much that there is a race, or a sec vuln or something. My point is that on my systems:

$HOME may be on NFS.
$HOME/.xsession-errors is a symlink to /usr/local/home/$USER/.xsession-errors
$HOME/.xsession-errors.old is a symlink to /usr/local/home/$USER/.xsession-errors.old

The obvious think is to just call readlink on logfile to canonicalize the filename.

The readlink manpage specifies a way to do this 'sort of' safely.

lstat(path) # to get the size of the symlink dest.
allocate string of correct size.
readlink(path)

Check that the path we got from readlink was size bytes.

Now obviously the link can be replaced (at readlink()) time, but readlink would return EINVAL then.
Once we have the 'canonical' name we can call rename on that.

Revision history for this message
Alec Warner (antarus) wrote :

I took a stab at this in the noted branch. my glib is terrible and It looks like we need to do this twice in session-child.c?

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

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