Comment 4 for bug 1839431

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 14:34:48.000000000 -0700
@@ -0,0 +1,47 @@
+From 0a21e0c7cd3eb35d79b31493da07df9736e0c6d5 Mon Sep 17 00:00:00 2001
+From: Vagrant Cascadian <email address hidden>
+Date: Sat, 19 Oct 2019 14:27:54 -0700
+Subject: [PATCH] Fix root access when LDM_USERNAME variable is unset.
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+This patch fixes an issue leading to root access on ltsp fat client
+systems when the user's shell is csh, tcsh, fish and probably any
+shell not supporting bourne shell syntax.
+
+When running in an unsupported shell, the LDM_USERNAME variable may
+end up being set to empty, resulting in a root login on ltsp fat
+clients.
+
+Mitigate this by skipping login when the LDM_USERNAME variable is
+unset.
+
+Thanks to Veeti Veteläinen for reporting the issue and Alkis
+Georgopoulos for further help in debugging.
+---
+ rc.d/X95-run-x-session | 8 ++++++--
+ 1 file changed, 6 insertions(+), 2 deletions(-)
+
+diff --git a/rc.d/X95-run-x-session b/rc.d/X95-run-x-session
+index b11bbf24..81f33d2e 100644
+--- a/rc.d/X95-run-x-session
++++ b/rc.d/X95-run-x-session
+@@ -40,8 +40,12 @@ if ! boolean_is_true "$LDM_NOMAD"; then
+ echo "CKCON_X11_DISPLAY_DEVICE DEFAULT=$CKCON_X11_DISPLAY_DEVICE" >> /etc/security/pam_env.conf
+ fi
+
+- # 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"
++ # Skip if LDM_USERNAME is unset, mitigating root access:
++ # 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
+
+ # Unmount any removable devices mounted by the user
+ for d in $(mount | awk '/uhelper=udisks/ {print $1}; /uhelper=devkit/ {print $1}'); do
+--
+2.20.1
+
diff -Nru ldm-2.18.06/debian/patches/series ldm-2.18.06/debian/patches/series
--- ldm-2.18.06/debian/patches/series 2018-06-07 14:51:28.000000000 -0700
+++ ldm-2.18.06/debian/patches/series 2019-10-19 14:35:03.000000000 -0700
@@ -1,3 +1,4 @@
 remove-hackish-gettext-function
 python3-shebang
 use-utf8-locale-to-generate-locale-lists
+Fix-root-access-when-LDM_USERNAME-variable-is-unset.patch