SYNERGY 512 SonarCloud vulnerabilities in Synergy-Core

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

src/gui/src/QUtility.cpp

@@ -22,6 +22,7 @@
 
 #if defined(Q_OS_LINUX)
 #include <QProcess>
+#include <QFile>
 #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;
-}

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();

src/lib/arch/unix/ArchMultithreadPosix.cpp

@@ -720,14 +720,16 @@ 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);
     }
 
     catch (XThreadCancel&) {
         // client called cancel()
+        // set base value
+        result = nullptr;
     }
     catch (...) {
         // note -- don't catch (...) to avoid masking bugs

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);
 

src/lib/net/SecureSocket.h

@@ -44,7 +44,7 @@ class SecureSocket : public TCPSocket {
     SecureSocket& operator=(SecureSocket &&) =delete;
 
     // ISocket overrides
-    void                close();
+    void                close() override;
 
     // IDataSocket overrides
     virtual void        connect(const NetworkAddress&);

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;
 }

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,21 +409,21 @@ 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<TCPSocket>(
                                 this, &TCPSocket::serviceConnecting,
                                 m_socket, m_readable, m_writable);
     }
     else {
         if (!(m_readable || (m_writable && (m_outputBuffer.getSize() > 0)))) {
-            return NULL;
+            return nullptr;
         }
         return new TSocketMultiplexerMethodJob<TCPSocket>(
                                 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;
 }