From 8a19b0d945db6ad3de98a64a0b307e8587c39d63 Mon Sep 17 00:00:00 2001
From: boyska <boyska@riseup.net>
Date: Thu, 9 Mar 2023 16:15:10 +0100
Subject: [PATCH] port exec-with-env to argparse

this is not just vanity: the previous parser could be abused to allow
privilege escalation from tails-persistent-storage to amnesia
---
 .../etc/sudoers.d/zzz_tps-frontend                 |  2 +-
 .../usr/local/lib/connect-drop-tps                 |  2 +-
 .../usr/local/lib/exec-with-env                    | 29 +++++++++++-----------
 .../usr/local/lib/exec-with-user-env               |  4 +--
 4 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/config/chroot_local-includes/etc/sudoers.d/zzz_tps-frontend b/config/chroot_local-includes/etc/sudoers.d/zzz_tps-frontend
index 027584d09f1..a5d20e7dd73 100644
--- a/config/chroot_local-includes/etc/sudoers.d/zzz_tps-frontend
+++ b/config/chroot_local-includes/etc/sudoers.d/zzz_tps-frontend
@@ -8,7 +8,7 @@ Defaults!TPS_CONNECT_DROP env_keep+="NOTIFY_SOCKET"
 # user so that connect-drop is able to authenticate to D-Bus as that user.
 amnesia   ALL = (tails-persistent-storage)    NOPASSWD: TPS_CONNECT_DROP
 
-Cmnd_Alias TPS_FRONTEND = /usr/local/lib/exec-with-env /tmp/*-env --delete /usr/local/lib/tps-frontend
+Cmnd_Alias TPS_FRONTEND = /usr/local/lib/exec-with-env --env-file /tmp/*-env --delete -- /usr/local/lib/tps-frontend
 
 Defaults!TPS_FRONTEND env_keep+="INHERIT_FD"
 Defaults!TPS_FRONTEND env_keep+="DESKTOP_STARTUP_ID"
diff --git a/config/chroot_local-includes/usr/local/lib/connect-drop-tps b/config/chroot_local-includes/usr/local/lib/connect-drop-tps
index 44104a5df41..c0a462921c4 100755
--- a/config/chroot_local-includes/usr/local/lib/connect-drop-tps
+++ b/config/chroot_local-includes/usr/local/lib/connect-drop-tps
@@ -11,5 +11,5 @@ ENVFILE="$1"
 
 exec /usr/local/lib/connect-drop --dbus --env-keep -- \
      sudo --close-from=4 -u amnesia \
-     /usr/local/lib/exec-with-env "${ENVFILE}" --delete \
+     /usr/local/lib/exec-with-env --env-file "${ENVFILE}" --delete -- \
      /usr/local/lib/tps-frontend
diff --git a/config/chroot_local-includes/usr/local/lib/exec-with-env b/config/chroot_local-includes/usr/local/lib/exec-with-env
index e14c1e40dbb..9b4438043c4 100755
--- a/config/chroot_local-includes/usr/local/lib/exec-with-env
+++ b/config/chroot_local-includes/usr/local/lib/exec-with-env
@@ -12,10 +12,18 @@
 
 import os
 import sys
+import argparse
 
 from tailslib.userenv import read_allowed_env_from_file, allowed_env
 
-usage = f"{sys.argv[0]} ENVFILE [--delete] [--] COMMAND [ARG]..."
+
+def get_parser():
+    p = argparse.ArgumentParser()
+    p.add_argument("--env-file", required=True)
+    p.add_argument("--delete", action='store_true', default=False)
+    p.add_argument('cmd', nargs='+')
+
+    return p
 
 
 def main():
@@ -23,27 +31,18 @@ def main():
         print(f"{sys.argv[0]}: This script must be run as amnesia", file=sys.stderr)
         sys.exit(1)
 
-    if len(sys.argv) < 3:
-        print(usage, file=sys.stderr)
-        sys.exit(1)
-
-    envfile = sys.argv[1]
-    env = read_allowed_env_from_file(envfile)
+    args = get_parser().parse_args()
+    env = read_allowed_env_from_file(args.env_file)
 
     # Update with allowed environment variables from the current
     # environment (because those should take precedence)
     env.update(allowed_env(os.environ))
 
-    if sys.argv[2] == "--delete":
-        os.remove(envfile)
-        del sys.argv[2]
-
-    if sys.argv[2] == "--":
+    if args.delete:
+        os.remove(args.env_file)
         del sys.argv[2]
 
-    file = sys.argv[2]
-    args = sys.argv[2:]
-    os.execvpe(file, args, env=env)
+    os.execvpe(args.cmd[0], args.cmd, env=env)
 
 
 if __name__ == "__main__":
diff --git a/config/chroot_local-includes/usr/local/lib/exec-with-user-env b/config/chroot_local-includes/usr/local/lib/exec-with-user-env
index 9b1421e7f60..3d86913a8d2 100755
--- a/config/chroot_local-includes/usr/local/lib/exec-with-user-env
+++ b/config/chroot_local-includes/usr/local/lib/exec-with-user-env
@@ -19,7 +19,7 @@ fi
 
 # Ensure that we're running as amnesia
 if [ "$(id -u)" -ne "1000"  ]; then
-  exec runuser -u amnesia -- /usr/local/lib/exec-with-env "${USER_ENV_FILE}" -- "$@"
+  exec runuser -u amnesia -- /usr/local/lib/exec-with-env --env-file "${USER_ENV_FILE}" -- "$@"
 else
-  exec /usr/local/lib/exec-with-env "${USER_ENV_FILE}" -- "$@"
+  exec /usr/local/lib/exec-with-env --env-file "${USER_ENV_FILE}" -- "$@"
 fi
-- 
2.11.4.GIT