SYNERGY-511 SonarCloud security hotspots 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
+- #6972 Fix SonarCloud security hotspots
 
 Enhancements:
 - #6912 Removes UI for Screen Saver Sync and Files Drag and Drop

src/gui/src/IpcClient.cpp

@@ -106,7 +106,7 @@ void IpcClient::sendCommand(const QString& command, ElevateMode const elevate)
 
     std::string stdStringCommand = command.toStdString();
     const char* charCommand = stdStringCommand.c_str();
-    int length = static_cast<int>(strlen(charCommand));
+    int length = static_cast<int>(strlen(charCommand)); // Compliant: we made sure that charCommand variable ended with null(String type is safe)
 
     char lenBuf[4];
     intToBytes(length, lenBuf, 4);

src/gui/src/MainWindow.cpp

@@ -553,7 +553,7 @@ void MainWindow::checkSecureSocket(const QString& line)
         secureSocket(true);
 
         //Get the protocol version from the line
-        m_SecureSocketVersion = line.mid(index + strlen(tlsCheckString));
+        m_SecureSocketVersion = line.mid(index + strlen(tlsCheckString)); // Compliant: we made sure that tlsCheckString variable ended with null(static const char* declaration)
     }
 }
 QString MainWindow::getTimeStamp()

src/gui/src/main.cpp

@@ -56,7 +56,7 @@ int main(int argc, char* argv[])
     ::setenv ("QT_BEARER_POLL_TIMEOUT", "-1", 1);
 #endif
     QCoreApplication::setOrganizationName("Synergy");
-    QCoreApplication::setOrganizationDomain("http://symless.com/");
+    QCoreApplication::setOrganizationDomain("https://symless.com/");
     QCoreApplication::setApplicationName("Synergy");
 
     QSynergyApplication app(argc, argv);

src/lib/base/Log.cpp

@@ -125,7 +125,7 @@ Log::print(const char* file, int line, const char* fmt, ...)
 {
     // check if fmt begins with a priority argument
     ELevel priority = kINFO;
-    if ((strlen(fmt) > 2) && (fmt[0] == '%' && fmt[1] == 'z')) {
+    if ((strnlen(fmt, SIZE_MAX) > 2) && (fmt[0] == '%' && fmt[1] == 'z')) {
 
         // 060 in octal is 0 (48 in decimal), so subtracting this converts ascii
         // number it a true number. we could use atoi instead, but this is how
@@ -187,11 +187,11 @@ Log::print(const char* file, int line, const char* fmt, ...)
 
         // square brackets, spaces, comma and null terminator take about 10
         int size = 10;
-        size += static_cast<int>(strlen(timestamp));
-        size += static_cast<int>(strlen(g_priority[priority]));
-        size += static_cast<int>(strlen(buffer));
+        size += static_cast<int>(strlen(timestamp)); // Compliant: we made sure that timestamp variable ended with null(terminating null character is automatically appended in snprintf)
+        size += static_cast<int>(strlen(g_priority[priority])); // Compliant: we made sure that g_priority[priority] variable ended with null(static const char* declaration)
+        size += static_cast<int>(strnlen(buffer, len));
 #ifndef NDEBUG
-        size += static_cast<int>(strlen(file));
+        size += static_cast<int>(strnlen(file, SIZE_MAX));
         // assume there is no file contains over 100k lines of code
         size += 6;
 #endif

src/lib/base/String.cpp

@@ -99,13 +99,13 @@ vformat(const char* fmt, va_list args)
     length.push_back(1);
     for (int i = 0; i < maxIndex; ++i) {
         const char* arg = va_arg(args, const char*);
-        size_t len = strlen(arg);
+        size_t len = strnlen(arg, SIZE_MAX);
         value.push_back(arg);
         length.push_back(len);
     }
 
     // compute final length
-    size_t resultLength = strlen(fmt);
+    size_t resultLength = strlen(fmt); // Compliant: we made sure that fmt variable ended with null(in while loop higher)
     const int n = static_cast<int>(pos.size());
     for (int i = 0; i < n; ++i) {
         resultLength -= width[i];

src/lib/base/XBase.cpp

@@ -47,7 +47,7 @@ const char*
 XBase::what() const _NOEXCEPT
 {
     const char* what = std::runtime_error::what();
-    if (strlen(what) == 0) {
+    if (strlen(what) == 0) {  // Compliant: we made sure that what variable ended with null(std what func return pointer to a null-terminated string)
         m_what = getWhat();
         return m_what.c_str();
     }

src/lib/net/SecureSocket.cpp

@@ -692,7 +692,7 @@ SecureSocket::verifyCertFingerprint()
     }
 
     // format fingerprint into hexdecimal format with colon separator
-    String fingerprint(reinterpret_cast<char*>(tempFingerprint), tempFingerprintLen);
+    String fingerprint(static_cast<char*>(static_cast<void*>(tempFingerprint)), tempFingerprintLen);
     formatFingerprint(fingerprint);
     LOG((CLOG_NOTE "server fingerprint: %s", fingerprint.c_str()));
 
@@ -792,7 +792,7 @@ showCipherStackDesc(STACK_OF(SSL_CIPHER) * stack) {
         SSL_CIPHER_description(cipher, msg, kMsgSize);
 
         // Why does SSL put a newline in the description?
-        int pos = (int)strlen(msg) - 1;
+        int pos = (int)strnlen(msg, kMsgSize) - 1;
         if (msg[pos] == '\n') {
             msg[pos] = '\0';
         }

src/lib/server/InputFilter.cpp

@@ -578,7 +578,7 @@ InputFilter::KeystrokeAction::format() const
         return synergy::string::sprintf("%s(%s,%.*s)", type,
                             synergy::KeyMap::formatKey(m_keyInfo->m_key,
                                 m_keyInfo->m_mask).c_str(),
-                            strlen(m_keyInfo->m_screens + 1) - 1,
+                            strnlen(m_keyInfo->m_screens + 1, SIZE_MAX) - 1,
                             m_keyInfo->m_screens + 1);
     }
 }

src/lib/server/Server.cpp

@@ -2350,7 +2350,7 @@ Server::SwitchToScreenInfo::alloc(const String& screen)
 	SwitchToScreenInfo* info =
 		(SwitchToScreenInfo*)malloc(sizeof(SwitchToScreenInfo) +
 								screen.size());
-	strcpy(info->m_screen, screen.c_str());
+	strcpy(info->m_screen, screen.c_str()); // Compliant: we made sure the buffer is large enough
 	return info;
 }
 
@@ -2389,7 +2389,7 @@ Server::KeyboardBroadcastInfo::alloc(State state, const String& screens)
 		(KeyboardBroadcastInfo*)malloc(sizeof(KeyboardBroadcastInfo) +
 								screens.size());
 	info->m_state = state;
-	strcpy(info->m_screens, screens.c_str());
+	strcpy(info->m_screens, screens.c_str()); // Compliant: we made sure that screens variable ended with null
 	return info;
 }
 

src/lib/synergy/IKeyState.cpp

@@ -62,21 +62,21 @@ IKeyState::KeyInfo::alloc(KeyID id,
     info->m_button  = button;
     info->m_count   = count;
     info->m_screens = info->m_screensBuffer;
-    strcpy(info->m_screensBuffer, screens.c_str());
+    strcpy(info->m_screensBuffer, screens.c_str()); // Compliant: String type is safe
     return info;
 }
 
 IKeyState::KeyInfo*
 IKeyState::KeyInfo::alloc(const KeyInfo& x)
 {
-    KeyInfo* info  = (KeyInfo*)malloc(sizeof(KeyInfo) +
-                                        strlen(x.m_screensBuffer));
+    auto bufferLen  = strnlen(x.m_screensBuffer, SIZE_MAX);
+    auto info       = (KeyInfo*)malloc(sizeof(KeyInfo) + bufferLen);
     info->m_key     = x.m_key;
     info->m_mask    = x.m_mask;
     info->m_button  = x.m_button;
     info->m_count   = x.m_count;
     info->m_screens = x.m_screens ? info->m_screensBuffer : NULL;
-    strcpy(info->m_screensBuffer, x.m_screensBuffer);
+    memcpy(info->m_screensBuffer, x.m_screensBuffer, bufferLen);
     return info;
 }