From 41c938eede3e9ea29ef86f3c25bab8fa6a51a509 Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Fri, 6 Jul 2012 13:45:38 +0200 Subject: [PATCH] IPC-server hardening and update - add IMPLEMENT_RANDOMIZE_STACK for ipcThread() - log / print boost interprocess exceptions - use MAX_URI_LENGTH in guiconstants.h (also used in qrcodedialog.cpp) - remove unneeded includes and ipcShutdown() from qtipcserver.cpp - fix a small mem-leak by deleting mq before re-using it - make ipcThread() and ipcThread2() static functions - add some more comments --- src/qt/bitcoin.cpp | 14 ++++++- src/qt/guiconstants.h | 4 +- src/qt/qtipcserver.cpp | 87 ++++++++++++++++++++++++++---------------- src/qt/qtipcserver.h | 7 +++- 4 files changed, 74 insertions(+), 38 deletions(-) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 55b5f74c3..f9f5115cd 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -126,13 +126,21 @@ int main(int argc, char *argv[]) const char *strURI = argv[i]; try { boost::interprocess::message_queue mq(boost::interprocess::open_only, BITCOINURI_QUEUE_NAME); - if(mq.try_send(strURI, strlen(strURI), 0)) + if (mq.try_send(strURI, strlen(strURI), 0)) + // if URI could be sent to the message queue exit here exit(0); else + // if URI could not be sent to the message queue do a normal Bitcoin-Qt startup break; } catch (boost::interprocess::interprocess_exception &ex) { - break; + // don't log the "file not found" exception, because that's normal for + // the first start of the first instance + if (ex.get_error_code() != boost::interprocess::not_found_error) + { + printf("main() - boost interprocess exception #%d: %s\n", ex.get_error_code(), ex.what()); + break; + } } } } @@ -278,6 +286,8 @@ int main(int argc, char *argv[]) mq.try_send(strURI, strlen(strURI), 0); } catch (boost::interprocess::interprocess_exception &ex) { + printf("main() - boost interprocess exception #%d: %s\n", ex.get_error_code(), ex.what()); + break; } } } diff --git a/src/qt/guiconstants.h b/src/qt/guiconstants.h index 6f6fa7f26..405ba396b 100644 --- a/src/qt/guiconstants.h +++ b/src/qt/guiconstants.h @@ -4,10 +4,10 @@ /* Milliseconds between model updates */ static const int MODEL_UPDATE_DELAY = 500; -/* Maximum passphrase length */ +/* AskPassphraseDialog -- Maximum passphrase length */ static const int MAX_PASSPHRASE_SIZE = 1024; -/* Size of icons in status bar */ +/* BitcoinGUI -- Size of icons in status bar */ static const int STATUSBAR_ICONSIZE = 16; /* Invalid field background style */ diff --git a/src/qt/qtipcserver.cpp b/src/qt/qtipcserver.cpp index f9ee9ad5c..e413c71bc 100644 --- a/src/qt/qtipcserver.cpp +++ b/src/qt/qtipcserver.cpp @@ -2,80 +2,96 @@ // Distributed under the MIT/X11 software license, see the accompanying // file license.txt or http://www.opensource.org/licenses/mit-license.php. -#include -#include -#include +#include "qtipcserver.h" +#include "guiconstants.h" +#include "ui_interface.h" +#include "util.h" + #include +#include #include #if defined(WIN32) && (!defined(BOOST_INTERPROCESS_HAS_WINDOWS_KERNEL_BOOTTIME) || !defined(BOOST_INTERPROCESS_HAS_KERNEL_BOOTTIME) || BOOST_VERSION < 104900) #warning Compiling without BOOST_INTERPROCESS_HAS_WINDOWS_KERNEL_BOOTTIME and BOOST_INTERPROCESS_HAS_KERNEL_BOOTTIME uncommented in boost/interprocess/detail/tmp_dir_helpers.hpp or using a boost version before 1.49 may have unintended results see svn.boost.org/trac/boost/ticket/5392 #endif -#include "ui_interface.h" -#include "qtipcserver.h" -#include "util.h" - +using namespace boost; using namespace boost::interprocess; using namespace boost::posix_time; -using namespace boost; -using namespace std; + +static void ipcThread2(void* pArg); #ifdef MAC_OSX // URI handling not implemented on OSX yet void ipcInit() { } -void ipcShutdown() { } #else -void ipcShutdown() +static void ipcThread(void* pArg) { - message_queue::remove(BITCOINURI_QUEUE_NAME); + IMPLEMENT_RANDOMIZE_STACK(ipcThread(pArg)); + + // Make this thread recognisable as the GUI-IPC thread + RenameThread("bitcoin-gui-ipc"); + + try + { + ipcThread2(pArg); + } + catch (std::exception& e) { + PrintExceptionContinue(&e, "ipcThread()"); + } catch (...) { + PrintExceptionContinue(NULL, "ipcThread()"); + } + printf("ipcThread exited\n"); } -void ipcThread(void* parg) +static void ipcThread2(void* pArg) { - // Make this thread recognisable as the GUI-IPC thread - RenameThread("bitcoin-gui-ipc"); + printf("ipcThread started\n"); + + message_queue* mq = (message_queue*)pArg; + char buffer[MAX_URI_LENGTH + 1] = ""; + size_t nSize = 0; + unsigned int nPriority = 0; - message_queue* mq = (message_queue*)parg; - char strBuf[257]; - size_t nSize; - unsigned int nPriority; loop { ptime d = boost::posix_time::microsec_clock::universal_time() + millisec(100); - if(mq->timed_receive(&strBuf, sizeof(strBuf), nSize, nPriority, d)) + if (mq->timed_receive(&buffer, sizeof(buffer), nSize, nPriority, d)) { - uiInterface.ThreadSafeHandleURI(std::string(strBuf, nSize)); + uiInterface.ThreadSafeHandleURI(std::string(buffer, nSize)); Sleep(1000); } + if (fShutdown) - { - ipcShutdown(); break; - } } - ipcShutdown(); + + // Remove message queue + message_queue::remove(BITCOINURI_QUEUE_NAME); + // Cleanup allocated memory + delete mq; } void ipcInit() { - message_queue* mq; - char strBuf[257]; - size_t nSize; - unsigned int nPriority; + message_queue* mq = NULL; + char buffer[MAX_URI_LENGTH + 1] = ""; + size_t nSize = 0; + unsigned int nPriority = 0; + try { - mq = new message_queue(open_or_create, BITCOINURI_QUEUE_NAME, 2, 256); + mq = new message_queue(open_or_create, BITCOINURI_QUEUE_NAME, 2, MAX_URI_LENGTH); // Make sure we don't lose any bitcoin: URIs for (int i = 0; i < 2; i++) { ptime d = boost::posix_time::microsec_clock::universal_time() + millisec(1); - if(mq->timed_receive(&strBuf, sizeof(strBuf), nSize, nPriority, d)) + if (mq->timed_receive(&buffer, sizeof(buffer), nSize, nPriority, d)) { - uiInterface.ThreadSafeHandleURI(std::string(strBuf, nSize)); + uiInterface.ThreadSafeHandleURI(std::string(buffer, nSize)); } else break; @@ -83,14 +99,19 @@ void ipcInit() // Make sure only one bitcoin instance is listening message_queue::remove(BITCOINURI_QUEUE_NAME); - mq = new message_queue(open_or_create, BITCOINURI_QUEUE_NAME, 2, 256); + delete mq; + + mq = new message_queue(open_or_create, BITCOINURI_QUEUE_NAME, 2, MAX_URI_LENGTH); } catch (interprocess_exception &ex) { + printf("ipcInit() - boost interprocess exception #%d: %s\n", ex.get_error_code(), ex.what()); return; } + if (!CreateThread(ipcThread, mq)) { delete mq; + return; } } diff --git a/src/qt/qtipcserver.h b/src/qt/qtipcserver.h index fcff10d8d..484b6222e 100644 --- a/src/qt/qtipcserver.h +++ b/src/qt/qtipcserver.h @@ -1,4 +1,9 @@ +#ifndef QTIPCSERVER_H +#define QTIPCSERVER_H + +// Define Bitcoin-Qt message queue name #define BITCOINURI_QUEUE_NAME "BitcoinURI" void ipcInit(); -void ipcShutdown(); + +#endif // QTIPCSERVER_H