From 271901c5cf06a2e3c00acc35759f0e022f9f16b9 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Fri, 12 May 2023 19:43:18 -0700 Subject: [PATCH] QProcess/Unix: add a simple way to reset the UID and GID for the child This is done as one of the last steps inside QProcess itself, so the child modifier and all other tasks still run with the parent process' permissions. On Linux, setting the UID to non-zero will also automatically clear the effective capabilities(7) set. This feature is only useful for setuid or setgid applications, so this commit updates the QCoreApplication::setSetuidAllowed() documentation to mention the QProcess flag. Change-Id: I3e3bfef633af4130a03afffd175e940c0668d244 Reviewed-by: Volker Hilsheimer Reviewed-by: Edward Welbourne --- src/corelib/io/qprocess.cpp | 6 ++++++ src/corelib/io/qprocess.h | 1 + src/corelib/io/qprocess_unix.cpp | 9 +++++++++ src/corelib/kernel/qcoreapplication.cpp | 5 ++++- .../io/qprocess/testUnixProcessParameters/main.cpp | 8 ++++++++ tests/auto/corelib/io/qprocess/tst_qprocess.cpp | 6 ++++++ 6 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/corelib/io/qprocess.cpp b/src/corelib/io/qprocess.cpp index 78d2de77793..a8a32ef633a 100644 --- a/src/corelib/io/qprocess.cpp +++ b/src/corelib/io/qprocess.cpp @@ -873,6 +873,12 @@ void QProcessPrivate::Channel::clear() terminate immediately; with this flag, the write operation fails without a signal and the child may continue executing. + \value [since 6.7] ResetIds Drops any retained, effective user or group + ID the current process may still have (see \c{setuid(2)} and + \c{setgid(2)}, plus QCoreApplication::setSetuidAllowed()). This is + useful if the current process was setuid or setgid and does not wish + the child process to retain the elevated privileges. + \value ResetSignalHandlers Resets all Unix signal handlers back to their default state (that is, pass \c SIG_DFL to \c{signal(2)}). This flag is useful to ensure any ignored (\c SIG_IGN) signal does not affect diff --git a/src/corelib/io/qprocess.h b/src/corelib/io/qprocess.h index 42bee0691b2..dc576ebc049 100644 --- a/src/corelib/io/qprocess.h +++ b/src/corelib/io/qprocess.h @@ -184,6 +184,7 @@ public: UseVFork = 0x0020, // like POSIX_SPAWN_USEVFORK CreateNewSession = 0x0040, // like POSIX_SPAWN_SETSID DisconnectControllingTerminal = 0x0080, + ResetIds = 0x0100, // like POSIX_SPAWN_RESETIDS }; Q_DECLARE_FLAGS(UnixProcessFlags, UnixProcessFlag) struct UnixProcessParameters diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp index 02438359f2c..91a745d5224 100644 --- a/src/corelib/io/qprocess_unix.cpp +++ b/src/corelib/io/qprocess_unix.cpp @@ -848,6 +848,15 @@ static const char *applyProcessParameters(const QProcess::UnixProcessParameters } } + // Apply UID and GID parameters last. This isn't expected to fail either: + // either we're trying to impersonate what we already are, or we're EUID or + // EGID root, in which case we are allowed to do this. + if (params.flags.testFlag(QProcess::UnixProcessFlag::ResetIds)) { + int r = setgid(getgid()); + r = setuid(getuid()); + (void) r; + } + return nullptr; } diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp index 3af761530f3..c8c3d7fe944 100644 --- a/src/corelib/kernel/qcoreapplication.cpp +++ b/src/corelib/kernel/qcoreapplication.cpp @@ -987,7 +987,10 @@ QCoreApplication::~QCoreApplication() and must be set before a QCoreApplication instance is created. \note It is strongly recommended not to enable this option since - it introduces security risks. + it introduces security risks. If this application does enable the flag and + starts child processes, it should drop the privileges as early as possible + by calling \c{setuid(2)} for itself, or at the latest by using the + QProcess::UnixProcessParameters::ResetIds flag. */ void QCoreApplication::setSetuidAllowed(bool allow) { diff --git a/tests/auto/corelib/io/qprocess/testUnixProcessParameters/main.cpp b/tests/auto/corelib/io/qprocess/testUnixProcessParameters/main.cpp index 962a1ee1c0f..bff3b920aa8 100644 --- a/tests/auto/corelib/io/qprocess/testUnixProcessParameters/main.cpp +++ b/tests/auto/corelib/io/qprocess/testUnixProcessParameters/main.cpp @@ -29,6 +29,14 @@ int main(int argc, char **argv) return EXIT_SUCCESS; } + if (cmd == "reset-ids") { + if (getuid() == geteuid() && getgid() == getegid()) + return EXIT_SUCCESS; + fprintf(stderr, "Real: %d %d; Effective: %d %d\n", + getuid(), getgid(), geteuid(), getegid()); + return EXIT_FAILURE; + } + if (cmd == "reset-sighand") { bool ok = true; diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp index 1e091d11785..8a927dc4fdc 100644 --- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp +++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp @@ -1733,6 +1733,7 @@ void tst_QProcess::unixProcessParameters_data() addRow("ignore-sigpipe", P::IgnoreSigPipe); addRow("file-descriptors", P::CloseFileDescriptors); addRow("setsid", P::CreateNewSession); + addRow("reset-ids", P::ResetIds); // On FreeBSD, we need to be session leader to disconnect from the CTTY addRow("noctty", P::DisconnectControllingTerminal | P::CreateNewSession); @@ -1784,6 +1785,11 @@ void tst_QProcess::unixProcessParameters() } } scope; + if (params.flags & QProcess::UnixProcessFlag::ResetIds) { + if (getuid() == geteuid() && getgid() == getegid()) + qInfo("Process has identical real and effective IDs; this test will do nothing"); + } + if (params.flags & QProcess::UnixProcessFlag::DisconnectControllingTerminal) { if (int fd = open("/dev/tty", O_RDONLY); fd < 0) { qInfo("Process has no controlling terminal; this test will do nothing");