diff --git a/ChangeLog b/ChangeLog index 7195c921b..728d2747a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,9 @@ v1.12.x-snapshot Bug fixes: - #6753 Fixed a number of vulnerabilities detected by static analysis - #6567 Fixed Synergy spawning hundreds of zombie processes +- #6755 Fixed Vulnerability in vreadf function detected by Static analysis +- #6758 Account name with space on windows causes synergy to error when starting +- #6760 Synergy loses license when creating a System scope config Enhancements: - #6750 Integrate SonarCloud for static analysis and test coverage diff --git a/src/gui/src/AppConfig.cpp b/src/gui/src/AppConfig.cpp index 029c326fc..f62afe0df 100644 --- a/src/gui/src/AppConfig.cpp +++ b/src/gui/src/AppConfig.cpp @@ -238,7 +238,6 @@ void AppConfig::loadSettings() m_ActivateEmail = loadSetting(kActivateEmail, "").toString(); m_CryptoEnabled = loadSetting(kCryptoEnabled, true).toBool(); m_AutoHide = loadSetting(kAutoHide, false).toBool(); - m_Serialkey = loadSetting(kSerialKey, "").toString().trimmed(); m_lastVersion = loadSetting(kLastVersion, "Unknown").toString(); m_LastExpiringWarningTime = loadSetting(kLastExpireWarningTime, 0).toInt(); m_ActivationHasRun = loadSetting(kActivationHasRun, false).toBool(); @@ -251,6 +250,16 @@ void AppConfig::loadSettings() m_ClientGroupChecked = loadSetting(kGroupClientCheck, true).toBool(); m_ServerHostname = loadSetting(kServerHostname).toString(); + //only change the serial key if the settings being loaded contains a key + bool updateSerial = GUI::Config::ConfigWriter::make() + ->hasSetting(settingName(kLoadSystemSettings),GUI::Config::ConfigWriter::kCurrent); + //if the setting exists and is not empty + updateSerial = updateSerial && !loadSetting(kSerialKey, "").toString().trimmed().isEmpty(); + + if (updateSerial) { + m_Serialkey = loadSetting(kSerialKey, "").toString().trimmed(); + } + //Set the default path of the TLS certificate file in the users DIR QString certificateFilename = QString("%1/%2/%3").arg(m_CoreInterface.getProfileDir(), "SSL", diff --git a/src/gui/src/MainWindow.cpp b/src/gui/src/MainWindow.cpp index 4cc156abb..2bcf21776 100644 --- a/src/gui/src/MainWindow.cpp +++ b/src/gui/src/MainWindow.cpp @@ -670,7 +670,7 @@ void MainWindow::startSynergy() if (m_AppConfig->getCryptoEnabled()) { args << "--enable-crypto"; - args << "--tls-cert" << m_AppConfig->getTLSCertPath(); + args << "--tls-cert" << QString("\"%1\"").arg(m_AppConfig->getTLSCertPath()); } #if defined(Q_OS_WIN) diff --git a/src/lib/synergy/ProtocolUtil.cpp b/src/lib/synergy/ProtocolUtil.cpp index 7d2c37ff8..9771c2379 100644 --- a/src/lib/synergy/ProtocolUtil.cpp +++ b/src/lib/synergy/ProtocolUtil.cpp @@ -111,146 +111,15 @@ ProtocolUtil::vreadf(synergy::IStream* stream, const char* fmt, va_list args) UInt32 len = eatLength(&fmt); switch (*fmt) { case 'i': { - // check for valid length - assert(len == 1 || len == 2 || len == 4); - - // read the data - UInt8 buffer[4]; - read(stream, buffer, len); - - // convert it - void* v = va_arg(args, void*); - switch (len) { - case 1: - // 1 byte integer - *static_cast(v) = buffer[0]; - LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", len, *static_cast(v), *static_cast(v))); - break; - - case 2: - // 2 byte integer - *static_cast(v) = - static_cast( - (static_cast(buffer[0]) << 8) | - static_cast(buffer[1])); - LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", len, *static_cast(v), *static_cast(v))); - break; - - case 4: - // 4 byte integer - *static_cast(v) = - (static_cast(buffer[0]) << 24) | - (static_cast(buffer[1]) << 16) | - (static_cast(buffer[2]) << 8) | - static_cast(buffer[3]); - LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", len, *static_cast(v), *static_cast(v))); - break; - } + readInt(stream, len, args); break; } - case 'I': { - // check for valid length - assert(len == 1 || len == 2 || len == 4); - - // read the vector length - UInt8 buffer[4]; - read(stream, buffer, 4); - UInt32 n = (static_cast(buffer[0]) << 24) | - (static_cast(buffer[1]) << 16) | - (static_cast(buffer[2]) << 8) | - static_cast(buffer[3]); - - // convert it - void* v = va_arg(args, void*); - switch (len) { - case 1: - // 1 byte integer - for (UInt32 i = 0; i < n; ++i) { - read(stream, buffer, 1); - static_cast*>(v)->push_back( - buffer[0]); - LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", len, i, static_cast*>(v)->back(), static_cast*>(v)->back())); - } - break; - - case 2: - // 2 byte integer - for (UInt32 i = 0; i < n; ++i) { - read(stream, buffer, 2); - static_cast*>(v)->push_back( - static_cast( - (static_cast(buffer[0]) << 8) | - static_cast(buffer[1]))); - LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", len, i, static_cast*>(v)->back(), static_cast*>(v)->back())); - } - break; - - case 4: - // 4 byte integer - for (UInt32 i = 0; i < n; ++i) { - read(stream, buffer, 4); - static_cast*>(v)->push_back( - (static_cast(buffer[0]) << 24) | - (static_cast(buffer[1]) << 16) | - (static_cast(buffer[2]) << 8) | - static_cast(buffer[3])); - LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", len, i, static_cast*>(v)->back(), static_cast*>(v)->back())); - } - break; - } + readVectorInt(stream, len, args); break; } - case 's': { - assert(len == 0); - - // read the string length - UInt8 buffer[128]; - read(stream, buffer, 4); - UInt32 len = (static_cast(buffer[0]) << 24) | - (static_cast(buffer[1]) << 16) | - (static_cast(buffer[2]) << 8) | - static_cast(buffer[3]); - - // use a fixed size buffer if its big enough - const bool useFixed = (len <= sizeof(buffer)); - - // allocate a buffer to read the data - UInt8* sBuffer = buffer; - if (!useFixed) { - try{ - sBuffer = new UInt8[len]; - } - catch (std::bad_alloc & exception) { - // Added try catch due to GHSA-chfm-333q-gfpp - LOG((CLOG_ERR "ALLOC: Unable to allocate memory %d bytes", len)); - LOG((CLOG_DEBUG "bad_alloc detected: Do you have enough free memory?")); - throw exception; - } - } - - // read the data - try { - read(stream, sBuffer, len); - } - catch (...) { - if (!useFixed) { - delete[] sBuffer; - } - throw; - } - - LOG((CLOG_DEBUG2 "readf: read %d byte string", len)); - - // save the data - String* dst = va_arg(args, String*); - dst->assign((const char*)sBuffer, len); - - // release the buffer - if (!useFixed) { - delete[] sBuffer; - } + readBytes(stream, len, args); break; } @@ -543,6 +412,155 @@ ProtocolUtil::read(synergy::IStream* stream, void* vbuffer, UInt32 count) } } +void ProtocolUtil::readInt(synergy::IStream * stream, UInt32 len, va_list args) { + // check for valid length + if (len == 4 || len == 2 || len == 1) { + + static const int buffer_size = 4; + // read the data + UInt8 buffer[buffer_size]; + //Read the buffer till the len or buffers_size, which ever is smaller + read(stream, buffer, len > buffer_size ? buffer_size : len); + + // convert it + void* v = va_arg(args, void*); + switch (len) { + case 1: + // 1 byte integer + *static_cast(v) = buffer[0]; + LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", len, *static_cast(v), *static_cast(v))); + break; + + case 2: + // 2 byte integer + *static_cast(v) = + static_cast( + (static_cast(buffer[0]) << 8) | + static_cast(buffer[1])); + LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", len, *static_cast(v), *static_cast(v))); + break; + + case 4: + // 4 byte integer + *static_cast(v) = + (static_cast(buffer[0]) << 24) | + (static_cast(buffer[1]) << 16) | + (static_cast(buffer[2]) << 8) | + static_cast(buffer[3]); + LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", len, *static_cast(v), *static_cast(v))); + break; + } + } + else { + //the length is wrong + LOG((CLOG_ERR "read: length to be read is wrong: '%d' should be 1,2, or 4", len)); + assert(false); //assert for debugging + } +} + +void ProtocolUtil::readVectorInt(synergy::IStream * stream, UInt32 len, va_list args) { + // check for valid length + assert(len == 1 || len == 2 || len == 4); + + // read the vector length + UInt8 buffer[4]; + read(stream, buffer, 4); + UInt32 n = (static_cast(buffer[0]) << 24) | + (static_cast(buffer[1]) << 16) | + (static_cast(buffer[2]) << 8) | + static_cast(buffer[3]); + + // convert it + void* v = va_arg(args, void*); + switch (len) { + case 1: + // 1 byte integer + for (UInt32 i = 0; i < n; ++i) { + read(stream, buffer, 1); + static_cast*>(v)->push_back( + buffer[0]); + LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", len, i, static_cast*>(v)->back(), static_cast*>(v)->back())); + } + break; + + case 2: + // 2 byte integer + for (UInt32 i = 0; i < n; ++i) { + read(stream, buffer, 2); + static_cast*>(v)->push_back( + static_cast( + (static_cast(buffer[0]) << 8) | + static_cast(buffer[1]))); + LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", len, i, static_cast*>(v)->back(), static_cast*>(v)->back())); + } + break; + + case 4: + // 4 byte integer + for (UInt32 i = 0; i < n; ++i) { + read(stream, buffer, 4); + static_cast*>(v)->push_back( + (static_cast(buffer[0]) << 24) | + (static_cast(buffer[1]) << 16) | + (static_cast(buffer[2]) << 8) | + static_cast(buffer[3])); + LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", len, i, static_cast*>(v)->back(), static_cast*>(v)->back())); + } + break; + } +} + +void ProtocolUtil::readBytes(synergy::IStream * stream, UInt32 len, va_list args) { + assert(len == 0); + + // read the string length + UInt8 buffer[128]; + read(stream, buffer, 4); + len = (static_cast(buffer[0]) << 24) | + (static_cast(buffer[1]) << 16) | + (static_cast(buffer[2]) << 8) | + static_cast(buffer[3]); + + // use a fixed size buffer if its big enough + const bool useFixed = (len <= sizeof(buffer)); + + // allocate a buffer to read the data + UInt8* sBuffer = buffer; + if (!useFixed) { + try{ + sBuffer = new UInt8[len]; + } + catch (std::bad_alloc & exception) { + // Added try catch due to GHSA-chfm-333q-gfpp + LOG((CLOG_ERR "ALLOC: Unable to allocate memory %d bytes", len)); + LOG((CLOG_DEBUG "bad_alloc detected: Do you have enough free memory?")); + throw exception; + } + } + + // read the data + try { + read(stream, sBuffer, len); + } + catch (...) { + if (!useFixed) { + delete[] sBuffer; + } + throw; + } + + LOG((CLOG_DEBUG2 "readf: read %d byte string", len)); + + // save the data + String* dst = va_arg(args, String*); + dst->assign((const char*)sBuffer, len); + + // release the buffer + if (!useFixed) { + delete[] sBuffer; + } +} + // // XIOReadMismatch diff --git a/src/lib/synergy/ProtocolUtil.h b/src/lib/synergy/ProtocolUtil.h index b7e43836f..04fad66e2 100644 --- a/src/lib/synergy/ProtocolUtil.h +++ b/src/lib/synergy/ProtocolUtil.h @@ -50,7 +50,7 @@ public: - \%s -- converts String* to stream of bytes - \%S -- converts integer N and const UInt8* to stream of N bytes */ - static void writef(synergy::IStream*, + static void writef(synergy::IStream*, const char* fmt, ...); //! Read formatted data @@ -69,19 +69,34 @@ public: - \%4I -- reads NBO 4 byte integers; arg is std::vector* - \%s -- reads bytes; argument must be a String*, \b not a char* */ - static bool readf(synergy::IStream*, + static bool readf(synergy::IStream*, const char* fmt, ...); private: - static void vwritef(synergy::IStream*, + static void vwritef(synergy::IStream*, const char* fmt, UInt32 size, va_list); - static void vreadf(synergy::IStream*, + static void vreadf(synergy::IStream*, const char* fmt, va_list); - static UInt32 getLength(const char* fmt, va_list); - static void writef(void*, const char* fmt, va_list); - static UInt32 eatLength(const char** fmt); - static void read(synergy::IStream*, void*, UInt32); + static UInt32 getLength(const char* fmt, va_list); + static void writef(void*, const char* fmt, va_list); + static UInt32 eatLength(const char** fmt); + static void read(synergy::IStream*, void*, UInt32); + + /** + * @brief Handles 1,2, or 4 byte Integers + */ + static void readInt(synergy::IStream*, UInt32, va_list); + + /** + * @brief Handles a Vector of integers + */ + static void readVectorInt(synergy::IStream*, UInt32, va_list); + + /** + * @brief Handles an array of bytes + */ + static void readBytes(synergy::IStream*, UInt32, va_list); }; //! Mismatched read exception