diff --git a/ChangeLog b/ChangeLog index 66ec45c16..0a7435ce1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -33,6 +33,7 @@ Bug fixes: - #6946 Add build stage to filename - #6950 Synergy client doesn't save the server address when rebooted - #6951 Fix issue creating standard version for Raspberry Pi +- #6971 Fix vulnerabilities from SonarCloud Enhancements: - #6912 Removes UI for Screen Saver Sync and Files Drag and Drop diff --git a/src/gui/src/QUtility.cpp b/src/gui/src/QUtility.cpp index 34c0db961..38bba3be1 100644 --- a/src/gui/src/QUtility.cpp +++ b/src/gui/src/QUtility.cpp @@ -22,6 +22,7 @@ #if defined(Q_OS_LINUX) #include +#include #endif #if defined(Q_OS_WIN) @@ -48,20 +49,6 @@ QString hash(const QString& string) return hash.toHex(); } -QString getFirstMacAddress() -{ - QString mac; - foreach (const QNetworkInterface &interface, QNetworkInterface::allInterfaces()) - { - mac = interface.hardwareAddress(); - if (mac.size() != 0) - { - break; - } - } - return mac; -} - qProcessorArch getProcessorArch() { #if defined(Q_OS_WIN) @@ -90,26 +77,3 @@ qProcessorArch getProcessorArch() return kProcessorArchUnknown; } - -QString getOSInformation() -{ - QString result; - -#if defined(Q_OS_LINUX) - result = "Linux"; - try { - QStringList arguments; - arguments.append("/etc/os-release"); - CommandProcess cp("/bin/cat", arguments); - QString output = cp.run(); - - QRegExp resultRegex(".*PRETTY_NAME=\"([^\"]+)\".*"); - if (resultRegex.exactMatch(output)) { - result = resultRegex.cap(1); - } - } catch (...) { - } -#endif - - return result; -} diff --git a/src/gui/src/QUtility.h b/src/gui/src/QUtility.h index 0738d96cc..56e76bb88 100644 --- a/src/gui/src/QUtility.h +++ b/src/gui/src/QUtility.h @@ -26,6 +26,4 @@ void setIndexFromItemData(QComboBox* comboBox, const QVariant& itemData); QString hash(const QString& string); -QString getFirstMacAddress(); qProcessorArch getProcessorArch(); -QString getOSInformation(); diff --git a/src/lib/arch/unix/ArchMultithreadPosix.cpp b/src/lib/arch/unix/ArchMultithreadPosix.cpp index 23a348aac..958a19cdf 100644 --- a/src/lib/arch/unix/ArchMultithreadPosix.cpp +++ b/src/lib/arch/unix/ArchMultithreadPosix.cpp @@ -720,7 +720,7 @@ ArchMultithreadPosix::doThreadFunc(ArchThread thread) lockMutex(m_threadMutex); unlockMutex(m_threadMutex); - void* result = NULL; + void* result = nullptr; try { // go result = (*thread->m_func)(thread->m_userData); @@ -728,6 +728,8 @@ ArchMultithreadPosix::doThreadFunc(ArchThread thread) catch (XThreadCancel&) { // client called cancel() + // set base value + result = nullptr; } catch (...) { // note -- don't catch (...) to avoid masking bugs diff --git a/src/lib/base/XBase.cpp b/src/lib/base/XBase.cpp index ee4b56655..cf51bc019 100644 --- a/src/lib/base/XBase.cpp +++ b/src/lib/base/XBase.cpp @@ -69,6 +69,7 @@ XBase::format(const char* /*id*/, const char* fmt, ...) const throw() } catch (...) { // ignore + result.clear(); } va_end(args); diff --git a/src/lib/net/SecureSocket.h b/src/lib/net/SecureSocket.h index d2d275fab..e20c133a2 100644 --- a/src/lib/net/SecureSocket.h +++ b/src/lib/net/SecureSocket.h @@ -44,7 +44,7 @@ public: SecureSocket& operator=(SecureSocket &&) =delete; // ISocket overrides - void close(); + void close() override; // IDataSocket overrides virtual void connect(const NetworkAddress&); diff --git a/src/lib/net/TCPListenSocket.cpp b/src/lib/net/TCPListenSocket.cpp index 707f8fe23..32f2dcf58 100644 --- a/src/lib/net/TCPListenSocket.cpp +++ b/src/lib/net/TCPListenSocket.cpp @@ -28,6 +28,7 @@ #include "mt/Mutex.h" #include "arch/Arch.h" #include "arch/XArch.h" +#include "base/Log.h" #include "base/IEventQueue.h" // @@ -57,6 +58,7 @@ TCPListenSocket::~TCPListenSocket() } catch (...) { // ignore + LOG((CLOG_WARN "error while closing TCP socket")); } delete m_mutex; } diff --git a/src/lib/net/TCPSocket.cpp b/src/lib/net/TCPSocket.cpp index bc7f28646..b9f226c73 100644 --- a/src/lib/net/TCPSocket.cpp +++ b/src/lib/net/TCPSocket.cpp @@ -47,7 +47,7 @@ TCPSocket::TCPSocket(IEventQueue* events, SocketMultiplexer* socketMultiplexer, try { m_socket = ARCH->newSocket(family, IArchNetwork::kSTREAM); } - catch (XArchNetwork& e) { + catch (const XArchNetwork& e) { throw XSocketCreate(e.what()); } @@ -64,7 +64,7 @@ TCPSocket::TCPSocket(IEventQueue* events, SocketMultiplexer* socketMultiplexer, m_flushed(&m_mutex, true), m_socketMultiplexer(socketMultiplexer) { - assert(m_socket != NULL); + assert(m_socket != nullptr); LOG((CLOG_DEBUG "Opening new socket: %08X", m_socket)); @@ -77,10 +77,11 @@ TCPSocket::TCPSocket(IEventQueue* events, SocketMultiplexer* socketMultiplexer, TCPSocket::~TCPSocket() { try { + // warning virtual function in destructor is very danger practice close(); } catch (...) { - // ignore + LOG((CLOG_DEBUG "error while TCP socket destruction")); } } @@ -90,10 +91,10 @@ TCPSocket::bind(const NetworkAddress& addr) try { ARCH->bindSocket(m_socket, addr.getAddress()); } - catch (XArchNetworkAddressInUse& e) { + catch (const XArchNetworkAddressInUse& e) { throw XSocketAddressInUse(e.what()); } - catch (XArchNetwork& e) { + catch (const XArchNetwork& e) { throw XSocketBind(e.what()); } } @@ -104,7 +105,7 @@ TCPSocket::close() LOG((CLOG_DEBUG "Closing socket: %08X", m_socket)); // remove ourself from the multiplexer - setJob(NULL); + setJob(nullptr); Lock lock(&m_mutex); @@ -115,13 +116,13 @@ TCPSocket::close() onDisconnected(); // close the socket - if (m_socket != NULL) { + if (m_socket != nullptr) { ArchSocket socket = m_socket; - m_socket = NULL; + m_socket = nullptr; try { ARCH->closeSocket(socket); } - catch (XArchNetwork& e) { + catch (const XArchNetwork& e) { // ignore, there's not much we can do LOG((CLOG_WARN "error closing socket: %s", e.what())); } @@ -143,7 +144,7 @@ TCPSocket::read(void* buffer, UInt32 n) if (n > size) { n = size; } - if (buffer != NULL && n != 0) { + if (buffer != nullptr && n != 0) { memcpy(buffer, m_inputBuffer.peek(n), n); } m_inputBuffer.pop(n); @@ -209,8 +210,9 @@ TCPSocket::shutdownInput() try { ARCH->closeSocketForRead(m_socket); } - catch (XArchNetwork&) { - // ignore + catch (const XArchNetwork& e) { + // ignore, there's not much we can do + LOG((CLOG_WARN "error closing socket: %s", e.what())); } // shutdown buffer for reading @@ -236,8 +238,9 @@ TCPSocket::shutdownOutput() try { ARCH->closeSocketForWrite(m_socket); } - catch (XArchNetwork&) { - // ignore + catch (const XArchNetwork& e) { + // ignore, there's not much we can do + LOG((CLOG_WARN "error closing socket: %s", e.what())); } // shutdown buffer for writing @@ -281,7 +284,7 @@ TCPSocket::connect(const NetworkAddress& addr) Lock lock(&m_mutex); // fail on attempts to reconnect - if (m_socket == NULL || m_connected) { + if (m_socket == nullptr || m_connected) { sendConnectionFailedEvent("busy"); return; } @@ -296,7 +299,7 @@ TCPSocket::connect(const NetworkAddress& addr) m_writable = true; } } - catch (XArchNetwork& e) { + catch (const XArchNetwork& e) { throw XSocketConnect(e.what()); } } @@ -317,13 +320,14 @@ TCPSocket::init() // mouse motion messages are much less useful if they're delayed. ARCH->setNoDelayOnSocket(m_socket, true); } - catch (XArchNetwork& e) { + catch (const XArchNetwork& e) { try { ARCH->closeSocket(m_socket); - m_socket = NULL; + m_socket = nullptr; } - catch (XArchNetwork&) { - // ignore + catch (const XArchNetwork& e) { + // ignore, there's not much we can do + LOG((CLOG_WARN "error closing socket: %s", e.what())); } throw XSocketCreate(e.what()); } @@ -392,7 +396,7 @@ void TCPSocket::setJob(ISocketMultiplexerJob* job) { // multiplexer will delete the old job - if (job == NULL) { + if (job == nullptr) { m_socketMultiplexer->removeSocket(this); } else { @@ -405,13 +409,13 @@ TCPSocket::newJob() { // note -- must have m_mutex locked on entry - if (m_socket == NULL) { - return NULL; + if (m_socket == nullptr) { + return nullptr; } else if (!m_connected) { assert(!m_readable); if (!(m_readable || m_writable)) { - return NULL; + return nullptr; } return new TSocketMultiplexerMethodJob( this, &TCPSocket::serviceConnecting, @@ -419,7 +423,7 @@ TCPSocket::newJob() } else { if (!(m_readable || (m_writable && (m_outputBuffer.getSize() > 0)))) { - return NULL; + return nullptr; } return new TSocketMultiplexerMethodJob( this, &TCPSocket::serviceConnected, @@ -439,7 +443,7 @@ TCPSocket::sendConnectionFailedEvent(const char* msg) void TCPSocket::sendEvent(Event::Type type) { - m_events->addEvent(Event(type, getEventTarget(), NULL)); + m_events->addEvent(Event(type, getEventTarget(), nullptr)); } void @@ -516,7 +520,7 @@ TCPSocket::serviceConnecting(ISocketMultiplexerJob* job, // connection may have failed or succeeded ARCH->throwErrorOnSocket(m_socket); } - catch (XArchNetwork& e) { + catch (const XArchNetwork& e) { sendConnectionFailedEvent(e.what()); onDisconnected(); return newJob(); @@ -592,5 +596,9 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job, } } - return result == kBreak ? NULL : result == kNew ? newJob() : job; + if (result == kBreak) { + return nullptr; + } + + return result == kNew ? newJob() : job; } diff --git a/src/lib/platform/XWindowsClipboard.cpp b/src/lib/platform/XWindowsClipboard.cpp index e0f6b19ef..17d4ed0c2 100644 --- a/src/lib/platform/XWindowsClipboard.cpp +++ b/src/lib/platform/XWindowsClipboard.cpp @@ -30,6 +30,7 @@ #include "base/Stopwatch.h" #include "common/stdvector.h" +#include #include #include #include @@ -70,7 +71,6 @@ XWindowsClipboard::XWindowsClipboard(Display* display, m_selection = XInternAtom(m_display, "CLIPBOARD", False); break; - case kClipboardSelection: default: m_selection = XA_PRIMARY; break; @@ -126,12 +126,10 @@ XWindowsClipboard::addRequest(Window owner, Window requestor, if (owner == m_window) { LOG((CLOG_DEBUG1 "request for clipboard %d, target %s by 0x%08x (property=%s)", m_selection, XWindowsUtil::atomToString(m_display, target).c_str(), requestor, XWindowsUtil::atomToString(m_display, property).c_str())); if (wasOwnedAtTime(time)) { - if (target == m_atomMultiple) { + if (target == m_atomMultiple && property != None) { // add a multiple request. property may not be None // according to ICCCM. - if (property != None) { - success = insertMultipleReply(requestor, time, property); - } + success = insertMultipleReply(requestor, time, property); } else { addSimpleRequest(requestor, target, time, property); @@ -178,7 +176,7 @@ XWindowsClipboard::addSimpleRequest(Window requestor, } else { IXWindowsClipboardConverter* converter = getConverter(target); - if (converter != NULL) { + if (converter != nullptr) { IClipboard::EFormat clipboardFormat = converter->getFormat(); if (m_added[clipboardFormat]) { try { @@ -188,6 +186,7 @@ XWindowsClipboard::addSimpleRequest(Window requestor, } catch (...) { // ignore -- cannot convert + LOG((CLOG_WARN "error while converting clipboard data")); } } } @@ -406,7 +405,7 @@ XWindowsClipboard::clearConverters() IXWindowsClipboardConverter* XWindowsClipboard::getConverter(Atom target, bool onlyIfNotAdded) const { - IXWindowsClipboardConverter* converter = NULL; + IXWindowsClipboardConverter* converter = nullptr; for (ConverterList::const_iterator index = m_converters.begin(); index != m_converters.end(); ++index) { converter = *index; @@ -414,17 +413,15 @@ XWindowsClipboard::getConverter(Atom target, bool onlyIfNotAdded) const break; } } - if (converter == NULL) { + if (converter == nullptr) { LOG((CLOG_DEBUG1 " no converter for target %s", XWindowsUtil::atomToString(m_display, target).c_str())); - return NULL; + return nullptr; } // optionally skip already handled targets - if (onlyIfNotAdded) { - if (m_added[converter->getFormat()]) { - LOG((CLOG_DEBUG1 " skipping handled format %d", converter->getFormat())); - return NULL; - } + if (onlyIfNotAdded && m_added[converter->getFormat()]) { + LOG((CLOG_DEBUG1 " skipping handled format %d", converter->getFormat())); + return nullptr; } return converter; @@ -519,7 +516,7 @@ XWindowsClipboard::icccmFillCache() } XWindowsUtil::convertAtomProperty(data); - const Atom* targets = reinterpret_cast(data.data()); // TODO: Safe? + auto targets = static_cast(static_cast(data.data())); const UInt32 numTargets = data.size() / sizeof(Atom); LOG((CLOG_DEBUG " available targets: %s", XWindowsUtil::atomsToString(m_display, targets, numTargets).c_str())); @@ -572,8 +569,8 @@ bool XWindowsClipboard::icccmGetSelection(Atom target, Atom* actualTarget, String* data) const { - assert(actualTarget != NULL); - assert(data != NULL); + assert(actualTarget != nullptr); + assert(data != nullptr); // request data conversion CICCCMGetClipboard getter(m_window, m_time, m_atomData); @@ -597,7 +594,7 @@ XWindowsClipboard::icccmGetTime() const String data; if (icccmGetSelection(m_atomTimestamp, &actualTarget, &data) && actualTarget == m_atomInteger) { - Time time = *reinterpret_cast(data.data()); + Time time = *static_cast(static_cast(data.data())); LOG((CLOG_DEBUG1 "got ICCCM time %d", time)); return time; } @@ -735,8 +732,7 @@ XWindowsClipboard::motifFillCache() // format list is after static item structure elements const SInt32 numFormats = item.m_numFormats - item.m_numDeletedFormats; - const SInt32* formats = reinterpret_cast(item.m_size + - static_cast(data.data())); + auto formats = static_cast(static_cast(item.m_size + data.data())); // get the available formats typedef std::map MotifFormatMap; @@ -832,7 +828,7 @@ XWindowsClipboard::motifGetSelection(const MotifClipFormat* format, Window root = RootWindow(m_display, DefaultScreen(m_display)); return XWindowsUtil::getWindowProperty(m_display, root, target, data, - actualTarget, NULL, False); + actualTarget, nullptr, False); } IClipboard::Time @@ -862,7 +858,7 @@ XWindowsClipboard::insertMultipleReply(Window requestor, // data is a list of atom pairs: target, property XWindowsUtil::convertAtomProperty(data); - const Atom* targets = reinterpret_cast(data.data()); + auto targets = static_cast(static_cast(data.data())); const UInt32 numTargets = data.size() / sizeof(Atom); // add replies for each target @@ -894,7 +890,7 @@ XWindowsClipboard::insertMultipleReply(Window requestor, void XWindowsClipboard::insertReply(Reply* reply) { - assert(reply != NULL); + assert(reply != nullptr); // note -- we must respond to requests in order if requestor,target,time // are the same, otherwise we can use whatever order we like with one @@ -995,7 +991,7 @@ XWindowsClipboard::pushReplies(ReplyMap::iterator& mapIndex, bool XWindowsClipboard::sendReply(Reply* reply) { - assert(reply != NULL); + assert(reply != nullptr); // bail out immediately if reply is done if (reply->m_done) { @@ -1083,68 +1079,75 @@ XWindowsClipboard::sendReply(Reply* reply) } } - // send notification if we haven't yet + // notification already sended if (!reply->m_replied) { - LOG((CLOG_DEBUG1 "clipboard: sending notify to 0x%08x,%d,%d", reply->m_requestor, reply->m_target, reply->m_property)); - reply->m_replied = true; + return false; + + } - // dump every property on the requestor window to the debug2 - // log. we've seen what appears to be a bug in lesstif and - // knowing the properties may help design a workaround, if - // it becomes necessary. - if (CLOG->getFilter() >= kDEBUG2) { - XWindowsUtil::ErrorLock lock(m_display); - int n; - Atom* props = XListProperties(m_display, reply->m_requestor, &n); - LOG((CLOG_DEBUG2 "properties of 0x%08x:", reply->m_requestor)); - for (int i = 0; i < n; ++i) { - Atom target; - String data; - char* name = XGetAtomName(m_display, props[i]); - if (!XWindowsUtil::getWindowProperty(m_display, - reply->m_requestor, - props[i], &data, &target, NULL, False)) { - LOG((CLOG_DEBUG2 " %s: ", name)); - } - else { - // if there are any non-ascii characters in string - // then print the binary data. - static const char* hex = "0123456789abcdef"; - String::size_type j = 0; - while (j < data.size()) { - if (data[j] < 32 || data[j] > 126) { - String tmp; - tmp.reserve(data.size() * 3); - for (j = 0; j < data.size(); ++j) { - unsigned char v = (unsigned char)data[j]; - tmp += hex[v >> 16]; - tmp += hex[v & 15]; - tmp += ' '; - } - data = tmp; - break; - } - ++j; - } - char* type = XGetAtomName(m_display, target); - LOG((CLOG_DEBUG2 " %s (%s): %s", name, type, data.c_str())); - if (type != NULL) { - XFree(type); - } - } - if (name != NULL) { - XFree(name); - } + LOG((CLOG_DEBUG1 "clipboard: sending notify to 0x%08x,%d,%d", reply->m_requestor, reply->m_target, reply->m_property)); + reply->m_replied = true; + + // nothing to log + if (CLOG->getFilter() < kDEBUG2) { + sendNotify(reply->m_requestor, m_selection, + reply->m_target, reply->m_property, + static_cast(reply->m_time)); + + // wait for delete notify + return false; + + } + + // dump every property on the requestor window to the debug2 + // log. we've seen what appears to be a bug in lesstif and + // knowing the properties may help design a workaround, if + // it becomes necessary. + XWindowsUtil::ErrorLock lock(m_display); + int n; + Atom* props = XListProperties(m_display, reply->m_requestor, &n); + LOG((CLOG_DEBUG2 "properties of 0x%08x:", reply->m_requestor)); + for (int i = 0; i < n; ++i) { + Atom target; + String data; + char* name = XGetAtomName(m_display, props[i]); + if (!XWindowsUtil::getWindowProperty(m_display, + reply->m_requestor, + props[i], &data, &target, nullptr, False)) { + LOG((CLOG_DEBUG2 " %s: ", name)); + } + else { + // convert to hex if contains non ascii symbols + if (std::find_if(data.begin(), data.end(), + [](const unsigned char& c) { return c < 32 || c > 126; }) != data.end()) { + const String hex_digits = "0123456789abcdef"; + String tmp; + tmp.reserve(data.length() * 3); + std::for_each(data.begin(), data.end(), [hex_digits, &tmp](const unsigned char& c) + { + tmp += hex_digits[c >> 16]; + tmp += hex_digits[c & 15]; + tmp += ' '; + }); + data = tmp; } - if (props != NULL) { - XFree(props); + char* type = XGetAtomName(m_display, target); + LOG((CLOG_DEBUG2 " %s (%s): %s", name, type, data.c_str())); + if (type != nullptr) { + XFree(type); } } - - sendNotify(reply->m_requestor, m_selection, - reply->m_target, reply->m_property, - reply->m_time); + if (name != nullptr) { + XFree(name); + } } + if (props != nullptr) { + XFree(props); + } + + sendNotify(reply->m_requestor, m_selection, + reply->m_target, reply->m_property, + reply->m_time); // wait for delete notify return false; @@ -1224,7 +1227,7 @@ XWindowsClipboard::wasOwnedAtTime(::Time time) const Atom XWindowsClipboard::getTargetsData(String& data, int* format) const { - assert(format != NULL); + assert(format != nullptr); // add standard targets XWindowsUtil::appendAtomData(data, m_atomTargets); @@ -1249,7 +1252,7 @@ XWindowsClipboard::getTargetsData(String& data, int* format) const Atom XWindowsClipboard::getTimestampData(String& data, int* format) const { - assert(format != NULL); + assert(format != nullptr); checkCache(); XWindowsUtil::appendTimeData(data, m_timeOwned); @@ -1271,8 +1274,6 @@ XWindowsClipboard::CICCCMGetClipboard::CICCCMGetClipboard( m_failed(false), m_done(false), m_reading(false), - m_data(NULL), - m_actualTarget(NULL), m_error(false) { // do nothing @@ -1287,8 +1288,8 @@ bool XWindowsClipboard::CICCCMGetClipboard::readClipboard(Display* display, Atom selection, Atom target, Atom* actualTarget, String* data) { - assert(actualTarget != NULL); - assert(data != NULL); + assert(actualTarget != nullptr); + assert(data != nullptr); LOG((CLOG_DEBUG1 "request selection=%s, target=%s, window=%x", XWindowsUtil::atomToString(display, selection).c_str(), XWindowsUtil::atomToString(display, target).c_str(), m_requestor)); @@ -1438,7 +1439,7 @@ XWindowsClipboard::CICCCMGetClipboard::processEvent( Atom target; const String::size_type oldSize = m_data->size(); if (!XWindowsUtil::getWindowProperty(display, m_requestor, - m_property, m_data, &target, NULL, True)) { + m_property, m_data, &target, nullptr, True)) { // unable to read property m_failed = true; return true; @@ -1448,11 +1449,7 @@ XWindowsClipboard::CICCCMGetClipboard::processEvent( // selection owner is busted. if the INCR property has no size // then the selection owner is busted. if (target == m_atomIncr) { - if (m_incr) { - m_failed = true; - m_error = true; - } - else if (m_data->size() == oldSize) { + if (m_incr || m_data->size() == oldSize) { m_failed = true; m_error = true; } diff --git a/src/lib/platform/XWindowsClipboard.h b/src/lib/platform/XWindowsClipboard.h index 66cf21fe9..fe088ffd2 100644 --- a/src/lib/platform/XWindowsClipboard.h +++ b/src/lib/platform/XWindowsClipboard.h @@ -172,11 +172,11 @@ private: bool m_reading; // the converted selection data - String* m_data; + String* m_data = nullptr; // the actual type of the data. if this is None then the // selection owner cannot convert to the requested type. - Atom* m_actualTarget; + Atom* m_actualTarget = nullptr; public: // true iff the selection owner didn't follow ICCCM conventions