[Spice-commits] 3 commits - common/vdcommon.cpp common/vdcommon.h vdagent/file_xfer.cpp vdagent/file_xfer.h

[Christophe Fergau]

 common/vdcommon.cpp   |   42 ++++++++++++++++++++++++++++++++++++++++++
 common/vdcommon.h     |   24 ++++++++++++++++++++++--
 vdagent/file_xfer.cpp |    4 ++--
 vdagent/file_xfer.h   |    2 +-
 4 files changed, 67 insertions(+), 5 deletions(-)

New commits:
commit 20d66f85ce587dbe3b6def3008f5226e43720e20
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Fri Feb 6 12:55:06 2015 +0100

    Remove MSVC++ strncpy/strcat fallbacks
    
    We currently have macros silently replacing use of strncpy/strcat with
    strcpy_s/strcat_s when using MSVC++. However, these macros can have
    unexpected effects as they use sizeof to find out the maximum size of
    the destination string. This is a very significant difference from
    strncpy/strcat, which can lead to subtle bugs as the behaviour is
    different between mingw and MSVC++.
    Now that we have our implementation of strcpy_s/strcat_s, we don't need
    these #defines.

diff --git a/common/vdcommon.h b/common/vdcommon.h
index 002ac23..93bb673 100644
--- a/common/vdcommon.h
+++ b/common/vdcommon.h
@@ -92,8 +92,6 @@ errno_t vdagent_strcpy_s(char *strDestination,
 
 #ifdef _MSC_VER // compiling with Visual Studio
 #define snprintf         sprintf_s
-#define strncpy(d,s,n)   strcpy_s(s, __min(n+1, sizeof(d)), s)
-#define strcat(d,s)      strcat_s(d, sizeof(d), s)
 #define sscanf           sscanf_s
 #endif
 
commit 5aa999320fe5c195ac951ad49d20029698c5536d
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Fri Feb 6 12:54:46 2015 +0100

    Use new vdagent_{strncpy,strcat}_s helpers

diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
index 8d7c86c..9e0bcda 100644
--- a/vdagent/file_xfer.cpp
+++ b/vdagent/file_xfer.cpp
@@ -87,8 +87,8 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage* start,
         return;
     }
 
-    strcat(file_path, "\\");
-    strcat(file_path, file_name);
+    vdagent_strcat_s(file_path, sizeof(file_path), "\\");
+    vdagent_strcat_s(file_path, sizeof(file_path), file_name);
     if((wlen = MultiByteToWideChar(CP_UTF8, 0, file_path, -1, NULL, 0)) == 0){
         vd_printf("failed getting WideChar length of %s", file_path);
         return;
diff --git a/vdagent/file_xfer.h b/vdagent/file_xfer.h
index 07a6808..7ac911e 100644
--- a/vdagent/file_xfer.h
+++ b/vdagent/file_xfer.h
@@ -27,7 +27,7 @@ typedef struct ALIGN_VC FileXferTask {
         // FIXME: should raise an error if name is too long..
         //        currently the only user is FileXfer::handle_start
         //        which verifies that strlen(_name) < MAX_PATH
-        strncpy(name, _name, sizeof(name) - 1);
+        vdagent_strcpy_s(name, sizeof(name), _name);
     }
     HANDLE handle;
     uint64_t size;
commit a4f082b0d12d24ff5821c93663e7cc99e07977e0
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Mon Feb 2 14:35:22 2015 +0100

    Add strcat_s/strcpy_s fallbacks
    
    These security functions are available when building with MSVC++. With
    mingw, they can be used at build time, but their availability will
    depend on the version of MSVCRT the user has installed on their system.
    In particular, a default install of Windows XP will not have a new
    enough MSVCRT version, causing runtime failures as the binary built with
    mingw and using strcat_s will not be able to find the necessary entry
    point in the MSVCRT runtime.
    
    This commit adds some strcat_s/strcpy_s-like functions used with mingw
    which will always be available.

diff --git a/common/vdcommon.cpp b/common/vdcommon.cpp
index 4dc50b4..4f80a2c 100644
--- a/common/vdcommon.cpp
+++ b/common/vdcommon.cpp
@@ -34,3 +34,45 @@ int supported_system_version()
     }
     return 0;
 }
+
+#ifndef HAVE_STRCAT_S
+errno_t vdagent_strcat_s(char *strDestination,
+                         size_t numberOfElements,
+                         const char *strSource)
+{
+    if (strDestination == NULL)
+        return EINVAL;
+    if (strSource == NULL) {
+        strDestination[0] = '\0';
+        return EINVAL;
+    }
+    if (strlen(strDestination) + strlen(strSource) + 1 > numberOfElements) {
+        strDestination[0] = '\0';
+        return ERANGE;
+    }
+
+    strcat(strDestination, strSource);
+
+    return 0;
+}
+#endif
+
+#ifndef HAVE_STRCPY_S
+errno_t vdagent_strcpy_s(char *strDestination,
+                         size_t numberOfElements,
+                         const char *strSource)
+{
+    if (strDestination == NULL)
+        return EINVAL;
+    strDestination[0] = '\0';
+    if (strSource == NULL)
+        return EINVAL;
+    if (strlen(strSource) + 1 > numberOfElements) {
+        return ERANGE;
+    }
+
+    strcpy(strDestination, strSource);
+
+    return 0;
+}
+#endif
diff --git a/common/vdcommon.h b/common/vdcommon.h
index af270db..002ac23 100644
--- a/common/vdcommon.h
+++ b/common/vdcommon.h
@@ -22,6 +22,7 @@
 #pragma warning(disable:4200)
 #endif
 
+#include <errno.h>
 #include <windows.h>
 #include "spice/vd_agent.h"
 #include "vdlog.h"
@@ -69,6 +70,27 @@ typedef CRITICAL_SECTION mutex_t;
 #endif /* OLDMSVCRT */
 
 #ifdef _MSC_VER // compiling with Visual Studio
+#define HAVE_STRCAT_S 1
+#define HAVE_STRCPY_S 1
+#endif
+
+#ifdef HAVE_STRCAT_S
+#define vdagent_strcat_s strcat_s
+#else
+errno_t vdagent_strcat_s(char *strDestination,
+                         size_t numberOfElements,
+                         const char *strSource);
+#endif
+
+#ifdef HAVE_STRCPY_S
+#define vdagent_strcpy_s strcpy_s
+#else
+errno_t vdagent_strcpy_s(char *strDestination,
+                         size_t numberOfElements,
+                         const char *strSource);
+#endif
+
+#ifdef _MSC_VER // compiling with Visual Studio
 #define snprintf         sprintf_s
 #define strncpy(d,s,n)   strcpy_s(s, __min(n+1, sizeof(d)), s)
 #define strcat(d,s)      strcat_s(d, sizeof(d), s)