diff --git a/ChangeLog b/ChangeLog index 0a7435ce1..b62958482 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,8 @@ Bug fixes: - #6953 | #6961 The system does not update server name in Server configuration - #6966 macOS 10.13 missing build number - #6968 System loses synchronisation between settings and configuration +- #6972 Fix SonarCloud security hotspots +- #6971 Fix vulnerabilities from SonarCloud Enhancements: - #6954 Move language selection to advanced section @@ -33,7 +35,6 @@ 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/IpcClient.cpp b/src/gui/src/IpcClient.cpp index 04ae39998..daa112b6c 100644 --- a/src/gui/src/IpcClient.cpp +++ b/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(strlen(charCommand)); + int length = static_cast(strlen(charCommand)); // Compliant: we made sure that charCommand variable ended with null(String type is safe) char lenBuf[4]; intToBytes(length, lenBuf, 4); diff --git a/src/gui/src/MainWindow.cpp b/src/gui/src/MainWindow.cpp index ddbf39595..5b486ae5b 100644 --- a/src/gui/src/MainWindow.cpp +++ b/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() diff --git a/src/lib/base/Log.cpp b/src/lib/base/Log.cpp index 0699a57db..ce24bd176 100644 --- a/src/lib/base/Log.cpp +++ b/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(strlen(timestamp)); - size += static_cast(strlen(g_priority[priority])); - size += static_cast(strlen(buffer)); + size += static_cast(strlen(timestamp)); // Compliant: we made sure that timestamp variable ended with null(terminating null character is automatically appended in snprintf) + size += static_cast(strlen(g_priority[priority])); // Compliant: we made sure that g_priority[priority] variable ended with null(static const char* declaration) + size += static_cast(strnlen(buffer, len)); #ifndef NDEBUG - size += static_cast(strlen(file)); + size += static_cast(strnlen(file, SIZE_MAX)); // assume there is no file contains over 100k lines of code size += 6; #endif diff --git a/src/lib/base/String.cpp b/src/lib/base/String.cpp index 53d04a13a..2dc3695ab 100644 --- a/src/lib/base/String.cpp +++ b/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(pos.size()); for (int i = 0; i < n; ++i) { resultLength -= width[i]; diff --git a/src/lib/base/XBase.cpp b/src/lib/base/XBase.cpp index cf51bc019..824aa101d 100644 --- a/src/lib/base/XBase.cpp +++ b/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(); } diff --git a/src/lib/net/SecureSocket.cpp b/src/lib/net/SecureSocket.cpp index fddc9f46c..6cb66bec1 100644 --- a/src/lib/net/SecureSocket.cpp +++ b/src/lib/net/SecureSocket.cpp @@ -692,7 +692,7 @@ SecureSocket::verifyCertFingerprint() } // format fingerprint into hexdecimal format with colon separator - String fingerprint(reinterpret_cast(tempFingerprint), tempFingerprintLen); + String fingerprint(static_cast(static_cast(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'; } diff --git a/src/lib/server/InputFilter.cpp b/src/lib/server/InputFilter.cpp index 7118c33cc..d2c4d526c 100644 --- a/src/lib/server/InputFilter.cpp +++ b/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); } } diff --git a/src/lib/server/Server.cpp b/src/lib/server/Server.cpp index 92faaf316..b620e401c 100644 --- a/src/lib/server/Server.cpp +++ b/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; } diff --git a/src/lib/synergy/IKeyState.cpp b/src/lib/synergy/IKeyState.cpp index a46fb64f5..4abb842ec 100644 --- a/src/lib/synergy/IKeyState.cpp +++ b/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 + 1); return info; }