Missing input sanitation in upstart logrotation cronjob

[halfdog]

Ubuntu Vivid 1504 (development branch) installs an insecure upstart logrotation script which will read user-supplied data from /run/user/[uid]/upstart/sessions and pass then unsanitized to an env command. As user run directory is user-writable, the user may inject arbitrary commands into the logrotation script, which will be executed during daily cron job execution around midnight with root privileges.

Problematic part of /etc/cron.daily/upstart:

for session in /run/user/*/upstart/sessions/*
do
    env $(cat $session) /sbin/initctl emit rotate-logs >/dev/null 2>&1 || true
done

On a system with e.g. libpam-systemd installed, standard login on TTY or via SSH will create the directory /run/user/[uid] writable to the user. By preparing a suitable session file, user supplied code will be run during the daily cron-jobs.

See [1] for more information.

# lsb_release -rd
Description: Ubuntu Vivid Vervet (development branch)
Release: 15.04

# apt-cache policy upstart-bin
upstart-bin:
  Installed: 1.13.2-0ubuntu7
  Candidate: 1.13.2-0ubuntu7
  Version table:
 *** 1.13.2-0ubuntu7 0
        500 http://archive.ubuntu.com/ubuntu/ vivid/main i386 Packages
        100 /var/lib/dpkg/status

[1] http://www.halfdog.net/Security/2015/UpstartLogrotationPrivilegeEscalation/

[James Hunt (jamesodhunt)]

Note - this problem only affects vivid fwics. Further, it does not affect Touch (since that uses Upstart as PID 1).

[James Hunt (jamesodhunt)]

Fixed in upstart 1.13.2-0ubuntu9, currently in the proposed pocket.

[Launchpad Janitor (janitor)]

This bug was fixed in the package upstart - 1.13.2-0ubuntu9

---------------
upstart (1.13.2-0ubuntu9) vivid; urgency=medium

  * debian/upstart-bin.upstart.cron.daily: Ensure the session uid matches that
    of the session file itself to stop a user forcing the logrotation of
    another users logs.
 -- James Hunt <email address hidden> Thu, 26 Feb 2015 14:35:41 +0000

[Jann Horn (jann-e)]

I'm not sure whether this is the right place to write this, but those permission checks look really racy - both the file type test and the file owner UID check.
(Besides, I think that an attacker should be able to hardlink a file created by another user into his directory, which would also lead to a bypass of the UID equality check, but I'm not familiar enough with ubuntu to be able to say whether that attack is actually feasible.)

[Seth Arnold (seth-arnold)]

Jann, thanks for the comment; I believe the checks aren't strictly necessary; the grep command used to extract one specific variable with the given legal values is the more important part of this patch.

That said, /run/user is a filesystem in its own right, so cross-mount hardlinks aren't possible, and at least on my system, all the directories are mode 700, so hardlinking to another user's file on the filesystem will be difficult.

Have you thought of anything else in the meantime? Thanks