From 452013e2097aa985bf8c3f8296d00d189401eea3 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 14 Dec 2009 15:30:48 +0100 Subject: more "obviously correct" code for secure opening of /dev/null Old code is not broken, new one merely looks "more obviously correct". Signed-off-by: Denys Vlasenko --- lib/Plugins/Firefox.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'lib/Plugins/Firefox.cpp') diff --git a/lib/Plugins/Firefox.cpp b/lib/Plugins/Firefox.cpp index 6f2c60d..9c10204 100644 --- a/lib/Plugins/Firefox.cpp +++ b/lib/Plugins/Firefox.cpp @@ -115,10 +115,7 @@ static pid_t ExecVP(char** pArgs, uid_t uid, std::string& pOutput) close(pipeout[0]); /* read side of the pipe */ xmove_fd(pipeout[1], STDOUT_FILENO); /* Make sure stdin is safely open to nothing */ - close(STDIN_FILENO); - if (open("/dev/null", O_RDONLY)) - if (open("/", O_RDONLY)) - abort(); /* never happens */ + xmove_fd(xopen("/dev/null", O_RDONLY), STDIN_FILENO); /* Not a good idea, we won't see any error messages */ /* close(STDERR_FILENO); */ @@ -692,8 +689,7 @@ static void InstallDebugInfos(const char *pDebugDumpDir, std::string& build_ids) { close(pipeout[0]); xmove_fd(pipeout[1], STDOUT_FILENO); - close(STDIN_FILENO); - xopen("/dev/null", O_RDONLY); + xmove_fd(xopen("/dev/null", O_RDONLY), STDIN_FILENO); /* Not a good idea, we won't see any error messages */ /*close(STDERR_FILENO);*/ -- cgit From 5f3b126f3013cb78fa2e5a8beb935021e21d5c5d Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 14 Dec 2009 16:00:28 +0100 Subject: add paranoia checks on setuid/setgid Signed-off-by: Denys Vlasenko --- lib/Plugins/Firefox.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) (limited to 'lib/Plugins/Firefox.cpp') diff --git a/lib/Plugins/Firefox.cpp b/lib/Plugins/Firefox.cpp index 9c10204..d9e6153 100644 --- a/lib/Plugins/Firefox.cpp +++ b/lib/Plugins/Firefox.cpp @@ -97,12 +97,6 @@ static pid_t ExecVP(char** pArgs, uid_t uid, std::string& pOutput) int pipeout[2]; pid_t child; - struct passwd* pw = getpwuid(uid); - if (!pw) - { - throw CABRTException(EXCEP_PLUGIN, "%s: can't get GID for UID", __func__); - } - xpipe(pipeout); child = fork(); if (child == -1) @@ -119,10 +113,11 @@ static pid_t ExecVP(char** pArgs, uid_t uid, std::string& pOutput) /* Not a good idea, we won't see any error messages */ /* close(STDERR_FILENO); */ - setgroups(1, &pw->pw_gid); - setregid(pw->pw_gid, pw->pw_gid); - setreuid(uid, uid); - setsid(); + struct passwd* pw = getpwuid(uid); + gid_t gid = pw ? pw->pw_gid : uid; + setgroups(1, &gid); + xsetregid(gid, gid); + xsetreuid(uid, uid); /* Nuke everything which may make setlocale() switch to non-POSIX locale: * we need to avoid having gdb output in some obscure language. @@ -856,8 +851,7 @@ static bool DebuginfoCheckPolkit(int uid) if (child_pid == 0) { //child - if (setuid(uid)) - exit(1); //paranoia + xsetreuid(uid, uid); PolkitResult result = polkit_check_authorization(getpid(), "org.fedoraproject.abrt.install-debuginfos"); exit(result != PolkitYes); //exit 1 (failure) if not allowed -- cgit From a0aa4bbdaa4d81ff54145af00a2c5a60671d3175 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 14 Dec 2009 16:04:04 +0100 Subject: a bit less verbose and more consistent log messages Signed-off-by: Denys Vlasenko --- lib/Plugins/Firefox.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'lib/Plugins/Firefox.cpp') diff --git a/lib/Plugins/Firefox.cpp b/lib/Plugins/Firefox.cpp index d9e6153..f350715 100644 --- a/lib/Plugins/Firefox.cpp +++ b/lib/Plugins/Firefox.cpp @@ -541,7 +541,7 @@ static void InstallDebugInfos(const char *pDebugDumpDir, std::string& build_ids) dd.LoadText(FILENAME_PACKAGE, package); } - update_client(_("Searching for debug-info packages...")); + update_client(_("Starting debuginfo installation")); int pipein[2], pipeout[2]; xpipe(pipein); @@ -603,8 +603,6 @@ Another application is holding the yum lock, cannot continue safe_write(pipein[1], "y\n", sizeof("y\n")-1); close(pipein[1]); - update_client(_("Downloading and installing debug-info packages...")); - FILE *pipeout_fp = fdopen(pipeout[0], "r"); if (pipeout_fp == NULL) /* never happens */ { @@ -668,7 +666,7 @@ Another application is holding the yum lock, cannot continue */ static void InstallDebugInfos(const char *pDebugDumpDir, std::string& build_ids) { - update_client(_("Searching for debug-info packages...")); + update_client(_("Starting debuginfo installation")); int pipeout[2]; //TODO: can we use ExecVP? xpipe(pipeout); @@ -701,8 +699,6 @@ static void InstallDebugInfos(const char *pDebugDumpDir, std::string& build_ids) close(pipeout[1]); - update_client(_("Downloading and installing debug-info packages...")); - FILE *pipeout_fp = fdopen(pipeout[0], "r"); if (pipeout_fp == NULL) /* never happens */ { -- cgit From a238ed9f6c80054028c6ba48a5fe773f8150c7c6 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 14 Dec 2009 16:05:06 +0100 Subject: another log message tweak Signed-off-by: Denys Vlasenko --- lib/Plugins/Firefox.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/Plugins/Firefox.cpp') diff --git a/lib/Plugins/Firefox.cpp b/lib/Plugins/Firefox.cpp index f350715..bcc5948 100644 --- a/lib/Plugins/Firefox.cpp +++ b/lib/Plugins/Firefox.cpp @@ -243,7 +243,7 @@ static int rate_backtrace(const char *backtrace) static void GetBacktrace(const char *pDebugDumpDir, std::string& pBacktrace) { - update_client(_("Getting backtrace...")); + update_client(_("Generating backtrace")); std::string UID; std::string executable; -- cgit