From 539da69a34711376a588e7c275d731fd89cc7e47 Mon Sep 17 00:00:00 2001 From: Jamie Newbon Date: Tue, 25 Aug 2020 19:16:21 +0100 Subject: [PATCH 1/3] #6753 Replaced vulnerable sprintf calls with snprintf --- src/lib/base/Log.cpp | 7 ++++--- src/lib/platform/XWindowsClipboard.cpp | 12 +++++++----- src/lib/synergy/App.cpp | 6 ++++-- src/lib/synergy/ClientApp.cpp | 7 ++++--- src/lib/synergy/ServerApp.cpp | 7 ++++--- 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/lib/base/Log.cpp b/src/lib/base/Log.cpp index d6eb336b6d..88984bf8e6 100644 --- a/src/lib/base/Log.cpp +++ b/src/lib/base/Log.cpp @@ -178,11 +178,12 @@ Log::print(const char* file, int line, const char* fmt, ...) if (priority != kPRINT) { struct tm *tm; - char timestamp[50]; + static const int timestamp_size = 50; + char timestamp[timestamp_size]; time_t t; time(&t); tm = localtime(&t); - sprintf(timestamp, "%04i-%02i-%02iT%02i:%02i:%02i", tm->tm_year + 1900, tm->tm_mon+1, tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec); + snprintf(timestamp, timestamp_size, "%04i-%02i-%02iT%02i:%02i:%02i", tm->tm_year + 1900, tm->tm_mon+1, tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec); // square brackets, spaces, comma and null terminator take about 10 int size = 10; @@ -197,7 +198,7 @@ Log::print(const char* file, int line, const char* fmt, ...) char* message = new char[size]; #ifndef NDEBUG - sprintf(message, "[%s] %s: %s\n\t%s,%d", timestamp, g_priority[priority], buffer, file, line); + snprintf(message, size, "[%s] %s: %s\n\t%s,%d", timestamp, g_priority[priority], buffer, file, line); #else sprintf(message, "[%s] %s: %s", timestamp, g_priority[priority], buffer); #endif diff --git a/src/lib/platform/XWindowsClipboard.cpp b/src/lib/platform/XWindowsClipboard.cpp index 5377504f43..e5273e7f27 100644 --- a/src/lib/platform/XWindowsClipboard.cpp +++ b/src/lib/platform/XWindowsClipboard.cpp @@ -710,8 +710,9 @@ XWindowsClipboard::motifFillCache() } // get the Motif item property from the root window - char name[18 + 20]; - sprintf(name, "_MOTIF_CLIP_ITEM_%d", header.m_item); + static const int buffer_size = 18 + 20; + char name[buffer_size]; + snprintf(name, buffer_size, "_MOTIF_CLIP_ITEM_%d", header.m_item); Atom atomItem = XInternAtom(m_display, name, False); data = ""; if (!XWindowsUtil::getWindowProperty(m_display, root, @@ -740,7 +741,7 @@ XWindowsClipboard::motifFillCache() MotifFormatMap motifFormats; for (SInt32 i = 0; i < numFormats; ++i) { // get Motif format property from the root window - sprintf(name, "_MOTIF_CLIP_ITEM_%d", formats[i]); + snprintf(name, buffer_size, "_MOTIF_CLIP_ITEM_%d", formats[i]); Atom atomFormat = XInternAtom(m_display, name, False); String data; if (!XWindowsUtil::getWindowProperty(m_display, root, @@ -822,8 +823,9 @@ XWindowsClipboard::motifGetSelection(const MotifClipFormat* format, // already stored on the root window and only if it fits in a // property. motif has some scheme for transferring part by // part that i don't know. - char name[18 + 20]; - sprintf(name, "_MOTIF_CLIP_ITEM_%d", format->m_data); + static const int buffer_size = 18+20; + char name[buffer_size]; + snprintf(name, buffer_size, "_MOTIF_CLIP_ITEM_%d", format->m_data); Atom target = XInternAtom(m_display, name, False); Window root = RootWindow(m_display, DefaultScreen(m_display)); return XWindowsUtil::getWindowProperty(m_display, root, diff --git a/src/lib/synergy/App.cpp b/src/lib/synergy/App.cpp index 93b39bf624..23d4fcf7d5 100644 --- a/src/lib/synergy/App.cpp +++ b/src/lib/synergy/App.cpp @@ -83,9 +83,11 @@ App::~App() void App::version() { - char buffer[500]; - sprintf( + static const int buffer_size = 500; + char buffer[buffer_size]; + snprintf( buffer, + buffer_size, "%s %s, protocol version %d.%d\n%s", argsBase().m_pname, kVersion, diff --git a/src/lib/synergy/ClientApp.cpp b/src/lib/synergy/ClientApp.cpp index 8582b8ccc0..b328215c71 100644 --- a/src/lib/synergy/ClientApp.cpp +++ b/src/lib/synergy/ClientApp.cpp @@ -118,10 +118,11 @@ ClientApp::help() # define WINAPI_ARG # define WINAPI_INFO #endif - - char buffer[2000]; - sprintf( + static const int buffer_size = 2000; + char buffer[buffer_size]; + snprintf( buffer, + buffer_size, "Usage: %s" " [--yscroll ]" WINAPI_ARG diff --git a/src/lib/synergy/ServerApp.cpp b/src/lib/synergy/ServerApp.cpp index 3c83510f6f..9abf757ae6 100644 --- a/src/lib/synergy/ServerApp.cpp +++ b/src/lib/synergy/ServerApp.cpp @@ -119,10 +119,11 @@ ServerApp::help() # define WINAPI_ARGS # define WINAPI_INFO #endif - - char buffer[3000]; - sprintf( + static const int buffer_size = 3000; + char buffer[buffer_size]; + snprintf( buffer, + buffer_size, "Usage: %s" " [--address
]" " [--config ]" From a2256edc06baddb4df1dcd8cd59dfb52a0afc385 Mon Sep 17 00:00:00 2001 From: Jamie Newbon Date: Tue, 25 Aug 2020 19:53:05 +0100 Subject: [PATCH 2/3] #6753 Updated ChangeLog --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index 5c29a9f19e..81a0ca3c25 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ v1.12.x-snapshot =========== +Bug fixes: +- #6753 Fixed a number of vulnerabilities detected by static analysis Enhancements: - #6750 Integrate SonarCloud for static analysis and test coverage From a21606664d383507d32c58eff7f9215e5477a061 Mon Sep 17 00:00:00 2001 From: Jamie Newbon Date: Tue, 25 Aug 2020 20:10:51 +0100 Subject: [PATCH 3/3] #6753 Updated missed sprintf --- src/lib/base/Log.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/base/Log.cpp b/src/lib/base/Log.cpp index 88984bf8e6..0699a57db7 100644 --- a/src/lib/base/Log.cpp +++ b/src/lib/base/Log.cpp @@ -200,7 +200,7 @@ Log::print(const char* file, int line, const char* fmt, ...) #ifndef NDEBUG snprintf(message, size, "[%s] %s: %s\n\t%s,%d", timestamp, g_priority[priority], buffer, file, line); #else - sprintf(message, "[%s] %s: %s", timestamp, g_priority[priority], buffer); + snprintf(message, size, "[%s] %s: %s", timestamp, g_priority[priority], buffer); #endif output(priority, message);