Two patches which fix: https://github.com/lxqt/lxqt-sudo/pull/42 Started at bsc#1100871 From 07ec9ec14e5d8ff2fe5aba33d9f0a1cd07a4db60 Mon Sep 17 00:00:00 2001 From: Palo Kisa Date: Mon, 12 Sep 2016 11:48:18 +0200 Subject: [PATCH] Sudo: Strip environment Leave only required environment variables (for X & locale) to get into the elevated child process. --- sudo.cpp | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/sudo.cpp b/sudo.cpp index 1530801..a98b75d 100644 --- a/sudo.cpp +++ b/sudo.cpp @@ -36,12 +36,14 @@ #include #include #include +#include #include #include #include #include #include #include +#include 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{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()); + } + } } } From 406a20279e24539e04cab1c96ff808b3e4e2d163 Mon Sep 17 00:00:00 2001 From: Palo Kisa Date: Tue, 24 Jul 2018 13:13:20 +0200 Subject: [PATCH] sudo: Force "C" locale for su/sudo We force the su/sudo to communicate with us in the simplest locale and then set the locale back for the command (by using the magic of shell interpretation). --- passworddialog.cpp | 9 +++------ passworddialog.h | 2 +- sudo.cpp | 44 ++++++++++++++++++++++++++------------------ sudo.h | 1 + 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/passworddialog.cpp b/passworddialog.cpp index fcd2208..6377752 100644 --- a/passworddialog.cpp +++ b/passworddialog.cpp @@ -4,7 +4,7 @@ * LXQt - a lightweight, Qt based, desktop toolset * https://lxqt.org * - * Copyright: 2015 LXQt team + * Copyright: 2015-2018 LXQt team * Authors: * Palo Kisa * @@ -29,7 +29,7 @@ #include "ui_passworddialog.h" #include -PasswordDialog::PasswordDialog(QStringList argv +PasswordDialog::PasswordDialog(const QString & cmd , QWidget * parent/* = 0*/ , Qt::WindowFlags f/* = 0*/) : QDialog(parent, f) @@ -37,10 +37,7 @@ PasswordDialog::PasswordDialog(QStringList argv { ui->setupUi(this); - ui->commandL->setText(argv.join(QStringLiteral(" "))); - QString cmd; - if (0 < argv.size()) - cmd = argv[0]; + ui->commandL->setText(cmd); ui->descriptionL->setText(tr("%1 needs administrative privileges.\nPlease enter your password.").arg(cmd)); ui->iconL->setPixmap(QIcon::fromTheme("dialog-password").pixmap(64, 64)); setWindowIcon(QIcon::fromTheme("security-high")); diff --git a/passworddialog.h b/passworddialog.h index 063b81a..d742a52 100644 --- a/passworddialog.h +++ b/passworddialog.h @@ -39,7 +39,7 @@ class PasswordDialog : public QDialog Q_OBJECT public: - PasswordDialog(QStringList argv + PasswordDialog(const QString & cmd , QWidget * parent = 0 , Qt::WindowFlags f = 0); ~PasswordDialog(); diff --git a/sudo.cpp b/sudo.cpp index f6002e1..1530801 100644 --- a/sudo.cpp +++ b/sudo.cpp @@ -4,7 +4,7 @@ * LXQt - a lightweight, Qt based, desktop toolset * https://lxqt.org * - * Copyright: 2015 LXQt team + * Copyright: 2015-2018 LXQt team * Authors: * Palo Kisa * @@ -141,16 +141,12 @@ int Sudo::main() //we were invoked through unknown link (or renamed binary) usage(tr("%1: no backend chosen!").arg(app_master)); return 1; - } else if (BACK_SU == mBackend && 1 < mArgs.size()) - { - QString cmd = mArgs.replaceInStrings(QRegExp(QStringLiteral("^(.*)$")), "'\\1'").join(QStringLiteral(" ")); - QTextStream(stderr) << tr("%1: warning - got multiple arguments for %2 backend, squashing into one: %3") - .arg(app_master).arg(su_prog).arg(cmd); - mArgs.erase(++mArgs.begin(), mArgs.end()); - mArgs[0] = std::move(cmd); } - mDlg.reset(new PasswordDialog{mArgs}); + mArgs.replaceInStrings(QStringLiteral("'"), QStringLiteral("'\\''")); + mSquashedArgs = mArgs.replaceInStrings(QRegExp(QStringLiteral("^(.*)$")), "'\\1'").join(QStringLiteral(" ")); + + mDlg.reset(new PasswordDialog{mSquashedArgs}); mDlg->setModal(true); lxqtApp->setActiveWindow(mDlg.data()); @@ -169,9 +165,8 @@ int Sudo::main() void Sudo::child() { - int params_cnt = 2 //1. su/sudo & last nullptr - + 1 //-c for su | -E for sudo - + mArgs.size(); + int params_cnt = 3 //1. su/sudo & "shell command" & last nullptr + + (BACK_SU == mBackend ? 1 : 3); //-c for su | -E /bin/sh -c for sudo std::unique_ptr params{new char const *[params_cnt]}; const char ** param_arg = params.get() + 1; @@ -179,20 +174,33 @@ void Sudo::child() if (BACK_SU == mBackend) { program = su_prog.toStdString(); - *(param_arg++) = "-c"; //run command } else { program = sudo_prog.toStdString(); *(param_arg++) = "-E"; //preserve environment + *(param_arg++) = "/bin/sh"; } + *(param_arg++) = "-c"; //run command params[0] = program.c_str(); - std::vector arguments; - for (const auto & a : mArgs) - arguments.push_back(a.toStdString()); - for (const auto & a : arguments) - *(param_arg++) = a.c_str(); + // Note: we force the su/sudo to communicate with us in the simplest + // locale and then set the locale back for the command + char const * const env_lc_all = getenv("LC_ALL"); + setenv("LC_ALL", "C", 1); + std::string command; + if (env_lc_all == nullptr) + { + command = "unset LC_ALL; "; + } else + { + command = "LC_ALL='"; + command += env_lc_all; + command += "' "; + } + command += "exec "; + command += mSquashedArgs.toStdString(); + *(param_arg++) = command.c_str(); *param_arg = nullptr; diff --git a/sudo.h b/sudo.h index c3eab94..d7a8c21 100644 --- a/sudo.h +++ b/sudo.h @@ -62,6 +62,7 @@ class Sudo : public QObject QScopedPointer mDlg; QStringList mArgs; backend_t mBackend; + QString mSquashedArgs; int mChildPid; int mPwdFd;