Merge branch 'master' of ssh://git.fedorahosted.org/git/abrt

3 files changed, 99 insertions, 92 deletions

diff --git a/src/Daemon/CommLayerServerDBus.cpp b/src/Daemon/CommLayerServerDBus.cpp
index fc28a12..cc98fc2 100644
--- a/src/Daemon/CommLayerServerDBus.cpp
+++ b/src/Daemon/CommLayerServerDBus.cpp
@@ -215,20 +215,19 @@ static int handle_Report(DBusMessage* call, DBusMessage* reply)
     comment = (it_comment != argin1.end()) ? it_comment->second[CD_CONTENT].c_str() : "";
     reproduce = (it_reproduce != argin1.end()) ? it_reproduce->second[CD_CONTENT].c_str() : "";
 
+    const char* errmsg = NULL;
     if (strlen(comment) > LIMIT_MESSAGE)
     {
-        dbus_message_unref(reply);
-        reply = dbus_message_new_error(call, DBUS_ERROR_FAILED, _("Comment message is too long"));
-        if (!reply)
-            die_out_of_memory();
-        send_flush_and_unref(reply);
-        return 0;
+        errmsg = _("Comment is too long");
     }
-
-    if (strlen(reproduce) > LIMIT_MESSAGE)
+    else if (strlen(reproduce) > LIMIT_MESSAGE)
+    {
+        errmsg = _("'How to reproduce' is too long");
+    }
+    if (errmsg)
     {
         dbus_message_unref(reply);
-        reply = dbus_message_new_error(call, DBUS_ERROR_FAILED, _("How to reproduce message is too long"));
+        reply = dbus_message_new_error(call, DBUS_ERROR_FAILED, errmsg);
         if (!reply)
             die_out_of_memory();
         send_flush_and_unref(reply);
@@ -247,21 +246,19 @@ static int handle_Report(DBusMessage* call, DBusMessage* reply)
         }
     }
 
-
     const char * sender = dbus_message_get_sender(call);
     if (!user_conf_data.empty())
     {
         std::string PluginName;
         map_plugin_settings_t plugin_settings;
-        map_map_string_t::const_iterator it_user_conf_data;
-        for (it_user_conf_data = user_conf_data.begin(); it_user_conf_data != user_conf_data.end(); it_user_conf_data++)
+        map_map_string_t::const_iterator it_user_conf_data = user_conf_data.begin();
+        for (; it_user_conf_data != user_conf_data.end(); it_user_conf_data++)
         {
-            map_string_t::const_iterator it_plugin_config;
-            map_string_t plugin_config = it_user_conf_data->second;
             PluginName = it_user_conf_data->first;
             plugin_settings = it_user_conf_data->second;
 #if DEBUG
             std::cout << "plugin name: " << it_user_conf_data->first;
+            map_string_t::const_iterator it_plugin_config;
             for    (it_plugin_config = it_user_conf_data->second.begin();
                     it_plugin_config != it_user_conf_data->second.end();
                     it_plugin_config++)
diff --git a/src/Daemon/MiddleWare.cpp b/src/Daemon/MiddleWare.cpp
index 9060d79..2ccd589 100644
--- a/src/Daemon/MiddleWare.cpp
+++ b/src/Daemon/MiddleWare.cpp
@@ -311,11 +311,12 @@ static bool CheckReport(const map_crash_report_t& pCrashReport)
     map_crash_report_t::const_iterator it_release = pCrashReport.find(FILENAME_RELEASE);
     map_crash_report_t::const_iterator it_executable = pCrashReport.find(FILENAME_EXECUTABLE);
 
-    if (it_analyzer == pCrashReport.end() || it_mwuid == pCrashReport.end() ||
-        it_mwuuid == pCrashReport.end() || it_package == pCrashReport.end() ||
-        it_architecture == pCrashReport.end() || it_kernel == pCrashReport.end() ||
-        it_component == pCrashReport.end() || it_release == pCrashReport.end() ||
-        it_executable == pCrashReport.end())
+    map_crash_report_t::const_iterator end = pCrashReport.end();
+    if (it_analyzer == end || it_mwuid == end ||
+        it_mwuuid == end || it_package == end ||
+        it_architecture == end || it_kernel == end ||
+        it_component == end || it_release == end ||
+        it_executable == end)
     {
         return false;
     }
@@ -336,9 +337,6 @@ report_status_t Report(const map_crash_report_t& pCrashReport,
                                     const std::string& pUID)
 {
     report_status_t ret;
-    std::string key;
-    std::string message;
-    CDebugDump dd;
 
     if (!CheckReport(pCrashReport))
     {
@@ -349,25 +347,28 @@ report_status_t Report(const map_crash_report_t& pCrashReport,
     std::string UID = pCrashReport.find(CD_MWUID)->second[CD_CONTENT];
     std::string UUID = pCrashReport.find(CD_MWUUID)->second[CD_CONTENT];
     std::string packageNVR = pCrashReport.find(FILENAME_PACKAGE)->second[CD_CONTENT];
-    std::string packageName = packageNVR.substr(0, packageNVR.rfind("-", packageNVR.rfind("-") - 1 ));
+    std::string packageName = packageNVR.substr(0, packageNVR.rfind("-", packageNVR.rfind("-") - 1));
 
     // Save comments and how to reproduciton
     map_crash_report_t::const_iterator it_comment = pCrashReport.find(CD_COMMENT);
     map_crash_report_t::const_iterator it_reproduce = pCrashReport.find(CD_REPRODUCE);
     std::string pDumpDir = getDebugDumpDir(UUID,UID);
 
-    dd.Open(pDumpDir);
-    if ( it_comment != pCrashReport.end() )
-    {
-        dd.SaveText(FILENAME_COMMENT, it_comment->second[CD_CONTENT]);
-    }
-    if ( it_reproduce != pCrashReport.end() )
     {
-        dd.SaveText(FILENAME_REPRODUCE, it_reproduce->second[CD_CONTENT]);
+        CDebugDump dd;
+        dd.Open(pDumpDir);
+        if (it_comment != pCrashReport.end())
+        {
+            dd.SaveText(FILENAME_COMMENT, it_comment->second[CD_CONTENT]);
+        }
+        if (it_reproduce != pCrashReport.end())
+        {
+            dd.SaveText(FILENAME_REPRODUCE, it_reproduce->second[CD_CONTENT]);
+        }
     }
-    dd.Close();
+
     // analyzer with package name (CCpp:xrog-x11-app) has higher priority
-    key = analyzer + ":" + packageName;
+    std::string key = analyzer + ":" + packageName;
     map_analyzer_actions_and_reporters_t::iterator keyPtr = s_mapAnalyzerActionsAndReporters.find(key);
     if (keyPtr == s_mapAnalyzerActionsAndReporters.end())
     {
@@ -375,6 +376,7 @@ report_status_t Report(const map_crash_report_t& pCrashReport,
         keyPtr = s_mapAnalyzerActionsAndReporters.find(analyzer);
     }
 
+    std::string message;
     if (keyPtr != s_mapAnalyzerActionsAndReporters.end())
     {
         vector_pair_string_string_t::iterator it_r = keyPtr->second.begin();
@@ -386,10 +388,10 @@ report_status_t Report(const map_crash_report_t& pCrashReport,
                 if (g_pPluginManager->GetPluginType(pluginName) == REPORTER)
                 {
                     CReporter* reporter = g_pPluginManager->GetReporter(pluginName);
+#if 0 /* Using ~user/.abrt/ is bad wrt security */
                     std::string home = "";
                     map_plugin_settings_t oldSettings;
                     map_plugin_settings_t newSettings;
-                    std::string res;
 
                     if (pUID != "")
                     {
@@ -404,14 +406,15 @@ report_status_t Report(const map_crash_report_t& pCrashReport,
                             }
                         }
                     }
+#endif
+                    std::string res = reporter->Report(pCrashReport, it_r->second);
 
-                    res = reporter->Report(pCrashReport, it_r->second);
-
+#if 0 /* Using ~user/.abrt/ is bad wrt security */
                     if (home != "")
                     {
                         reporter->SetSettings(oldSettings);
                     }
-
+#endif
                     ret[pluginName].push_back("1");
                     ret[pluginName].push_back(res);
                     message += res + "\n";
diff --git a/src/Daemon/PluginManager.cpp b/src/Daemon/PluginManager.cpp
index bd5727e..2363308 100644
--- a/src/Daemon/PluginManager.cpp
+++ b/src/Daemon/PluginManager.cpp
@@ -376,66 +376,73 @@ void CPluginManager::SetPluginSettings(const std::string& pName,
                                        const map_plugin_settings_t& pSettings)
 {
     map_abrt_plugins_t::iterator abrt_plugin = m_mapABRTPlugins.find(pName);
-    if (abrt_plugin != m_mapABRTPlugins.end())
+    if (abrt_plugin == m_mapABRTPlugins.end())
     {
-        map_plugins_t::iterator plugin = m_mapPlugins.find(pName);
-        if (plugin != m_mapPlugins.end())
-        {
-            plugin->second->SetSettings(pSettings);
+        return;
+    }
+    map_plugins_t::iterator plugin = m_mapPlugins.find(pName);
+    if (plugin == m_mapPlugins.end())
+    {
+        return;
+    }
+    plugin->second->SetSettings(pSettings);
 
-            if (abrt_plugin->second->GetType() == REPORTER)
-            {
-                std::string home = get_home_dir(atoi(pUID.c_str()));
-                if (home != "")
-                {
-                    std::string confDir = home + "/.abrt";
-                    std::string confPath = confDir + "/" + pName + "."PLUGINS_CONF_EXTENSION;
-                    uid_t uid = atoi(pUID.c_str());
-                    struct passwd* pw = getpwuid(uid);
-                    gid_t gid = pw ? pw->pw_gid : uid;
-
-                    struct stat buf;
-                    if (stat(confDir.c_str(), &buf) != 0)
-                    {
-                        if (mkdir(confDir.c_str(), 0700) == -1)
-                        {
-                            perror_msg("Can't create dir '%s'", confDir.c_str());
-                            return;
-                        }
-                        if (chmod(confDir.c_str(), 0700) == -1)
-                        {
-                            perror_msg("Can't change mod of dir '%s'", confDir.c_str());
-                            return;
-                        }
-                        if (chown(confDir.c_str(), uid, gid) == -1)
-                        {
-                            perror_msg("Can't change '%s' ownership to %u:%u", confPath.c_str(), (int)uid, (int)gid);
-                            return;
-                        }
-
-                    }
-                    else if (!S_ISDIR(buf.st_mode))
-                    {
-                        perror_msg("'%s' is not a directory", confDir.c_str());
-                        return;
-                    }
-
-                    /** we don't want to save it from daemon if it's running under root
-                    but wi might get back to this once we make the daemon to not run
-                    with root privileges
-                    */
-                    /*
-                    SavePluginSettings(confPath, pSettings);
-                    if (chown(confPath.c_str(), uid, gid) == -1)
-                    {
-                        perror_msg("Can't change '%s' ownership to %u:%u", confPath.c_str(), (int)uid, (int)gid);
-                        return;
-                    }
-                    */
-                }
-            }
+#if 0 /* Writing to ~user/.abrt/ is bad wrt security */
+    if (abrt_plugin->second->GetType() != REPORTER)
+    {
+        return;
+    }
+
+    std::string home = get_home_dir(atoi(pUID.c_str()));
+    if (home == "")
+    {
+        return;
+    }
+
+    std::string confDir = home + "/.abrt";
+    std::string confPath = confDir + "/" + pName + "."PLUGINS_CONF_EXTENSION;
+    uid_t uid = atoi(pUID.c_str());
+    struct passwd* pw = getpwuid(uid);
+    gid_t gid = pw ? pw->pw_gid : uid;
+
+    struct stat buf;
+    if (stat(confDir.c_str(), &buf) != 0)
+    {
+        if (mkdir(confDir.c_str(), 0700) == -1)
+        {
+            perror_msg("Can't create dir '%s'", confDir.c_str());
+            return;
+        }
+        if (chmod(confDir.c_str(), 0700) == -1)
+        {
+            perror_msg("Can't change mod of dir '%s'", confDir.c_str());
+            return;
+        }
+        if (chown(confDir.c_str(), uid, gid) == -1)
+        {
+            perror_msg("Can't change '%s' ownership to %u:%u", confPath.c_str(), (int)uid, (int)gid);
+            return;
         }
     }
+    else if (!S_ISDIR(buf.st_mode))
+    {
+        perror_msg("'%s' is not a directory", confDir.c_str());
+        return;
+    }
+
+    /** we don't want to save it from daemon if it's running under root
+    but wi might get back to this once we make the daemon to not run
+    with root privileges
+    */
+    /*
+    SavePluginSettings(confPath, pSettings);
+    if (chown(confPath.c_str(), uid, gid) == -1)
+    {
+        perror_msg("Can't change '%s' ownership to %u:%u", confPath.c_str(), (int)uid, (int)gid);
+        return;
+    }
+    */
+#endif
 }
 
 map_plugin_settings_t CPluginManager::GetPluginSettings(const std::string& pName,