ldm logs users with non-default login shell in as root

[Veeti Veteläinen]

Steps to reproduce:
1. Change a user's login shell to /usr/bin/* (for ex. /usr/bin/fish) on the ltsp server
2. Login on a client as that user.
3. You should be logged in as root.

I didn't test if this happens with other shells than bash in /bin

[Vagrant Cascadian (vagrantc)]

Tested on Debian Buster. Seems to only happen on a fat client with some
shells. On thin clients, it simply fails to log in. Possibly related to:

  https://bugs.debian.org/490897

Confirmed behavior logging in as root:

  fish, tcsh, csh

Following shells log in as the correct user:

  zsh, dash, /bin/sh (as dash), bash, mksh, ksh

Login hangs due to some unrelated problem:

  sash

With some shells, the LDM_USERNAME environment variable remains unset,
and so in /usr/share/ldm/rc.d/X95-run-x-session ends up calling:

  su - ${LDM_USERNAME} ...

And ends up as root.

The quick fix would be to check if LDM_USERNAME is set and at least
error out if so, rather than granting root.

Still would be better to identify exactly where in the code we're
expecting LDM_USERNAME to be set and fix it or error out there.

live well,
  vagrant

[Alkis Georgopoulos (alkisg)]

uumas gave me remote and I was able to have a quick look.

The problems start in /usr/share/ldm/rc.d/X01-localapps, at the following lines:
# Get user information from the server
IFS=':' read -r LDM_USERNAME dummy pw_uid pw_gid pw_gecos LDM_HOME pw_shell <<EOF
$(ssh_run '/usr/bin/getent passwd $(/usr/bin/id -u)')
EOF

Fish doesn't use $() but () to execute commands in a subshell, so the line above destroys LDM_USERNAME instead of getting additional info.
$(ssh_run "/usr/bin/getent passwd $LDM_USERNAME")
could be used instead, but this might fail in some cases.

The important thing is to put a test above the `su - ${LDM_USERNAME}` line, to log an error and exit if LDM_USERNAME isn't set.

[Marc Deslauriers (mdeslaur)]

Thanks for taking the time to report this bug and helping to make Ubuntu better. Since the package referred to in this bug is in universe or multiverse, it is community maintained. If you are able, I suggest coordinating with upstream and posting a debdiff for this issue. When a debdiff is available, members of the security team will review it and publish the package. See the following link for more information: https://wiki.ubuntu.com/SecurityTeam/UpdateProcedures

[Vagrant Cascadian (vagrantc)]

submitted to the debian security team:

On 2019-09-23, Vagrant Cascadian wrote:
> A user of LDM reported that users with a non-bournish shell (fish) that
> LDM logs the user in as root when LDM is used with LTSP booted as a fat
> client. I've also confirmed the behavior with the csh and tcsh shells.
> This most likely affects all versions of ldm in Debian; I've confirmed
> on buster. Since a user can change their shell, any system with one of
> these shells (or another triggering shell) available would potentially
> be vulnerable.
>
> This is due to a command run LDM's local apps hooks provided by LTSP; it
> essentially runs an ssh command to the server and fails to parse the
> output, resulting in an empty LDM_USERNAME variable, which is then later
> called with "su - ${LDM_USERNAME} ..." resulting in a root login on the
> client. Looking over this, the whole idea of how LDM works is a bit
> frightening...
>
> The fix/workaround is to error out if LDM_USERNAME isn't set; this will
> still fail to login for fish, csh or tcsh (or possibly other shells),
> but at least it will fail rather than give a root login.
...
> This is documented in a private bug on launchpad:
>
> https://bugs.launchpad.net/ltsp/+bug/1839431

I've finally gotten a chance to test the mitigation and have a debdiff
to propose below.

The ldm versions in oldoldstable (2:2.2.15-2), oldstable (2:2.2.18-2)
and stable/testing/unstable (2:2.18.06-1) are all likely vulnerable,
though I have only tested and confirmed the fix on stable.

This part of the code hasn't changed much, so I suspect the patch would
also apply on older versions, possibly unchanged or with very minor
adjustments.

This doesn't fix logins for the triggering shells, but that will never
be fixed, as upstream LTSP has been rewritten from scratch and no longer
uses LDM or has anything resembling the affected code. We haven't pushed
any changes to upstream git repository yet, but upstream is unlikely to
make a new release in any case.

There's also the possibility of additionally mitigating this from the
LTSP side, which is where the code exists that unsets LDM_USERNAME,
triggering the issue... but I think mitigating it in LDM is the most
urgent fix needed.

Please let me know how to proceed.

Thanks!

live well,
  vagrant

diff -Nru ldm-2.18.06/debian/changelog ldm-2.18.06/debian/changelog
--- ldm-2.18.06/debian/changelog 2018-06-07 15:09:48.000000000 -0700
+++ ldm-2.18.06/debian/changelog 2019-10-19 14:38:32.000000000 -0700
@@ -1,3 +1,9 @@
+ldm (2:2.18.06-1+deb10u1) buster-security; urgency=medium
+
+ * Add patch fixing root access when LDM_USERNAME is unset.
+
+ -- Vagrant Cascadian <email address hidden> Sat, 19 Oct 2019 14:38:32 -0700
+
 ldm (2:2.18.06-1) unstable; urgency=medium

   * New upstream version.
diff -Nru ldm-2.18.06/debian/patches/Fix-root-access-when-LDM_USERNAME-variable-is-unset.patch ldm-2.18.06/debian/patches/Fix-root-access-when-LDM_USERNAME-variable-is-unset.patch
--- ldm-2.18.06/debian/patches/Fix-root-access-when-LDM_USERNAME-variable-is-unset.patch 1969-12-31 16:00:00.000000000 -0800
+++ ldm-2.18.06/debian/patches/Fix-root-access-when-LDM_USERNAME-variable-is-unset.patch 2019-10-19 ...

[Seth Arnold (seth-arnold)]

On Mon, Oct 21, 2019 at 08:46:56PM -0000, Vagrant Cascadian wrote:
> ++ # https://bugs.launchpad.net/ubuntu/+source/ldm/+bug/1839431
> ++ if [ -n "${LDM_USERNAME}" ]; then
> ++ # The XDG_* variables are for logind support.
> ++ XDG_SEAT=${XDG_SEAT:-seat0} XDG_VTNR=${SCREEN_NUM#0} su - ${LDM_USERNAME} -c "$CLIENT_ENV $MY_LANG DISPLAY=$DISPLAY ICEAUTHORITY=$ICEAUTHORITY XAUTHORITY=$XAUTHORITY $LDM_XSESSION $LDM_SESSION"
> ++ fi

This probably gets really exciting if the LDM_USERNAME includes any shell
metacharacters. How about the other variables? I can't recall shell
quoting rules well enough to guess offhand how they'll influence the
execution of this command line.

Thanks

[Alkis Georgopoulos (alkisg)]

In su - ${LDM_USERNAME}, if LDM_USERNAME is unquoted, then it can be used to pass parameters to su (like an additional -c parameter which fortunately su would ignore), or it can cause problems if it contains globs, that would expand to filenames.

su - "${LDM_USERNAME}" ... is safer; in general unquoted shell variables should only be used for specific purposes.

Quoting isn't necessary in assignments, so XDG_SEAT etc aren't an issue.

[Vagrant Cascadian (vagrantc)]

A mitigating fix has been pushed upstream:

  https://git.launchpad.net/~ltsp-upstream/ltsp/+git/ldm/commit/?id=c351ac69ef63ed6c84221cef73e409059661b8ba

Patched packages have been uploaded to Debian buster/stable and stretch/oldstable.

[Vagrant Cascadian (vagrantc)]

A mitigating fix has been pushed upstream:

  https://git.launchpad.net/~ltsp-upstream/ltsp/+git/ldm/commit/?id=c351ac69ef63ed6c84221cef73e409059661b8ba

Patched packages have been uploaded to Debian buster/stable and stretch/oldstable, though are not yet published.

[Alkis Georgopoulos (alkisg)]

Closing old LTSP bugs as they're no longer relevant after LTSP has been rewritten from scratch.