From 3a7b636f42e4694b8d3b6aa27153db6b007e1e14 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Wed, 24 Aug 2011 10:30:06 +0100 Subject: [PATCH 1/2] Fix possible out-of-bounds array access if an empty command-line argument is passed to ProcessUtils::runElevated() on Windows --- src/ProcessUtils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ProcessUtils.cpp b/src/ProcessUtils.cpp index fa23a97..11183ce 100644 --- a/src/ProcessUtils.cpp +++ b/src/ProcessUtils.cpp @@ -280,7 +280,7 @@ void ProcessUtils::runElevatedWindows(const std::string& executable, { std::string arg = *iter; - if (arg.at(0) != '"' && arg.at(arg.size()-1) != '"') + if (!arg.empty() && arg.at(0) != '"' && arg.at(arg.size()-1) != '"') { arg.insert(0,"\""); arg.append("\""); From f37d4690785437b35f4def8d2e313a7f0727d13f Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Wed, 24 Aug 2011 11:23:48 +0100 Subject: [PATCH 2/2] Enable -Wall -Werror when building on Linux/Mac and fix several bugs discovered in the process. * Fix ProcessUtils::runAsyncUnix() not returning a value * Fix UpdateDialogGtk::updateRetryCancel() not returning a value * Add missing includes in TestUpdaterOptions.cpp * Fix unsigned/signed int comparisons --- src/CMakeLists.txt | 4 ++++ src/ProcessUtils.cpp | 7 ++++--- src/ProcessUtils.h | 2 +- src/UpdateDialogGtk.cpp | 1 + src/UpdateInstaller.cpp | 3 +-- src/UpdaterOptions.cpp | 2 +- src/tests/TestUpdaterOptions.cpp | 3 +++ 7 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 3e43bc4..61d5be9 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -3,6 +3,10 @@ add_subdirectory(tests) find_package(Threads REQUIRED) +if (UNIX) + add_definitions(-Wall -Werror) +endif() + include(UsePkgConfig) pkgconfig(gtk+-2.0 GTK2_INCLUDE_DIR GTK2_LINK_DIR GTK2_LINK_FLAGS GTK2_CFLAGS) diff --git a/src/ProcessUtils.cpp b/src/ProcessUtils.cpp index fa23a97..956de14 100644 --- a/src/ProcessUtils.cpp +++ b/src/ProcessUtils.cpp @@ -45,7 +45,7 @@ int ProcessUtils::runSync(const std::string& executable, int ProcessUtils::runSyncUnix(const std::string& executable, const std::list& args) { - int pid = runAsyncUnix(executable,args); + PLATFORM_PID pid = runAsyncUnix(executable,args); int status = 0; waitpid(pid,&status,0); return status; @@ -124,7 +124,7 @@ void ProcessUtils::runElevatedLinux(const std::string& executable, sudos.push_back("gksudo"); sudos.push_back("gksu"); - for (int i=0; i < sudos.size(); i++) + for (unsigned int i=0; i < sudos.size(); i++) { const std::string& sudoBinary = sudos.at(i); @@ -312,7 +312,7 @@ void ProcessUtils::runElevatedWindows(const std::string& executable, #endif #ifdef PLATFORM_UNIX -int ProcessUtils::runAsyncUnix(const std::string& executable, +PLATFORM_PID ProcessUtils::runAsyncUnix(const std::string& executable, const std::list& args) { pid_t child = fork(); @@ -339,6 +339,7 @@ int ProcessUtils::runAsyncUnix(const std::string& executable, { LOG(Info,"Started child process " + intToStr(child)); } + return child; } #endif diff --git a/src/ProcessUtils.h b/src/ProcessUtils.h index 9d2ba5c..1919950 100644 --- a/src/ProcessUtils.h +++ b/src/ProcessUtils.h @@ -31,7 +31,7 @@ class ProcessUtils static void runElevatedWindows(const std::string& executable, const std::list& args); - static int runAsyncUnix(const std::string& executable, + static PLATFORM_PID runAsyncUnix(const std::string& executable, const std::list& args); static void runAsyncWindows(const std::string& executable, const std::list& args); diff --git a/src/UpdateDialogGtk.cpp b/src/UpdateDialogGtk.cpp index d1daed8..8ec1a53 100644 --- a/src/UpdateDialogGtk.cpp +++ b/src/UpdateDialogGtk.cpp @@ -108,6 +108,7 @@ void UpdateDialogGtk::updateError(const std::string& errorMessage) bool UpdateDialogGtk::updateRetryCancel(const std::string& message) { // TODO + return false; } void UpdateDialogGtk::updateProgress(int percentage) diff --git a/src/UpdateInstaller.cpp b/src/UpdateInstaller.cpp index e0c152c..43b7abe 100644 --- a/src/UpdateInstaller.cpp +++ b/src/UpdateInstaller.cpp @@ -7,8 +7,8 @@ UpdateInstaller::UpdateInstaller() : m_mode(Setup) -, m_script(0) , m_waitPid(0) +, m_script(0) , m_observer(0) { } @@ -269,7 +269,6 @@ void UpdateInstaller::removeBackups() std::map::const_iterator iter = m_backups.begin(); for (;iter != m_backups.end();iter++) { - const std::string& installedFile = iter->first; const std::string& backupFile = iter->second; FileOps::removeFile(backupFile.c_str()); } diff --git a/src/UpdaterOptions.cpp b/src/UpdaterOptions.cpp index 869fd06..13b15e5 100644 --- a/src/UpdaterOptions.cpp +++ b/src/UpdaterOptions.cpp @@ -43,7 +43,7 @@ UpdateInstaller::Mode stringToMode(const std::string& modeStr) void UpdaterOptions::parseOldFormatArg(const std::string& arg, std::string* key, std::string* value) { - int pos = arg.find('='); + unsigned int pos = arg.find('='); if (pos != std::string::npos) { *key = arg.substr(0,pos); diff --git a/src/tests/TestUpdaterOptions.cpp b/src/tests/TestUpdaterOptions.cpp index 6d6f7c8..21dd8e8 100644 --- a/src/tests/TestUpdaterOptions.cpp +++ b/src/tests/TestUpdaterOptions.cpp @@ -3,6 +3,9 @@ #include "TestUtils.h" #include "UpdaterOptions.h" +#include +#include + void TestUpdaterOptions::testOldFormatArgs() { const int argc = 6;