Strip environment in lxqt-sudo to leave only required environment variables to get into the elevated child process.

Summary: Strip environment variables in lxqt-sudo

Test Plan:
1. Test first with lxqt-sudo from repo: lxqt-sudo env 2> env.txt
2. Install this version
3. Run lxqt-sudo env 2> env2.txt
4. Compare env.txt with env2.txt: diff -y env.txt env2.txt
5. Both files should differ in most environment variables, such as HOME

Reviewers: tsimonq2, wxl

Reviewed By: tsimonq2

Differential Revision: https://phab.lubuntu.me/D44
ubuntu/focal
apt-ghetto 6 years ago committed by Simon Quigley
parent 103dd3e937
commit 068d5bf369

6
debian/changelog vendored

@ -1,3 +1,9 @@
lxqt-sudo (0.13.0-0ubuntu3) UNRELEASED; urgency=medium
* Leave only required variables to get into the elevated child process.
-- apt-ghetto <apt-ghetto@protonmail.com> Wed, 14 Nov 2018 17:55:39 +0100
lxqt-sudo (0.13.0-0ubuntu2) cosmic; urgency=medium lxqt-sudo (0.13.0-0ubuntu2) cosmic; urgency=medium
* Change Uploaders to Ubuntu uploaders. * Change Uploaders to Ubuntu uploaders.

@ -0,0 +1 @@
lxqt-sudo

@ -1 +1,2 @@
fix-layout-line-break.patch fix-layout-line-break.patch
sudo-strip-environment.patch

@ -0,0 +1,69 @@
Description: Sudo: Strip environment
Leave only required environment variables (for X & locale) to get into the elevated child process.
Author: Palo Kisa <palo.kisa@gmail.com>
Applied-Upstream: https://github.com/lxqt/lxqt-sudo/commit/07ec9ec14e5d8ff2fe5aba33d9f0a1cd07a4db60
---
This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
--- a/sudo.cpp
+++ b/sudo.cpp
@@ -36,12 +36,14 @@
#include <QSocketNotifier>
#include <QDebug>
#include <QThread>
+#include <QProcessEnvironment>
#include <pty.h>
#include <unistd.h>
#include <memory>
#include <csignal>
#include <sys/wait.h>
#include <fcntl.h>
+#include <iostream>
namespace
{
@@ -80,11 +82,42 @@ namespace
<< QObject::tr("%1 version %2\n").arg(app_master).arg(app_version);
}
+ //Note: array must be sorted to allow usage of binary search
+ static constexpr char const * const ALLOWED_VARS[] = {
+ "DISPLAY"
+ , "LANG", "LANGUAGE", "LC_ADDRESS", "LC_ALL", "LC_COLLATE", "LC_CTYPE", "LC_IDENTIFICATION", "LC_MEASUREMENT"
+ , "LC_MESSAGES", "LC_MONETARY", "LC_NAME", "LC_NUMERIC", "LC_PAPER", "LC_TELEPHONE", "LC_TIME"
+ , "PATH", "QT_PLATFORM_PLUGIN", "QT_QPA_PLATFORMTHEME", "WAYLAND_DISPLAY", "XAUTHORITY"
+ };
+ static constexpr char const * const * const ALLOWED_END = ALLOWED_VARS + sizeof (ALLOWED_VARS) / sizeof (ALLOWED_VARS[0]);
+ struct assert_helper
+ {
+ assert_helper()
+ {
+ Q_ASSERT(std::is_sorted(ALLOWED_VARS, ALLOWED_END
+ , [] (char const * const a, char const * const b) { return strcmp(a, b) < 0; }));
+ }
+ };
+ assert_helper h;
+
inline void env_workarounds()
{
- //cleanup environment
- //pcmanfm-qt will not start if the DBUS_SESSION_BUS_ADDRESS is preserved
- unsetenv("DBUS_SESSION_BUS_ADDRESS");
+ std::cerr << LXQTSUDO << ": Stripping child environment except for: ";
+ std::copy(ALLOWED_VARS, ALLOWED_END - 1, std::ostream_iterator<const char *>{std::cerr, ", "});
+ std::cerr << *(ALLOWED_END - 1) << '\n'; // printing the last separately to avoid trailing comma
+ // cleanup environment, because e.g.:
+ // - pcmanfm-qt will not start if the DBUS_SESSION_BUS_ADDRESS is preserved
+ // - Qt apps may change user's config files permissions if the XDG_* are preserved
+ for (auto const & key : QProcessEnvironment::systemEnvironment().keys())
+ {
+ auto const & i = std::lower_bound(ALLOWED_VARS, ALLOWED_END, key, [] (char const * const a, QString const & b) {
+ return b > a;
+ });
+ if (i == ALLOWED_END || key != *i)
+ {
+ unsetenv(key.toStdString().c_str());
+ }
+ }
}
}
Loading…
Cancel
Save