[Spice-devel] [vdagent-win v2 PATCH 4/6] vdagent: file_xfer: make g_key_get_string safer

[Uri Lublin]

By providing the size of the destination string buffer.
---
 vdagent/file_xfer.cpp | 25 +++++++++++++++++++------
 vdagent/file_xfer.h   |  3 ++-
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
index eb2119a..e402eb2 100644
--- a/vdagent/file_xfer.cpp
+++ b/vdagent/file_xfer.cpp
@@ -49,7 +49,7 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage* start,

     status->id = start->id;
     status->result = VD_AGENT_FILE_XFER_STATUS_ERROR;
-    if (!g_key_get_string(file_meta, "vdagent-file-xfer", "name", file_name) ||
+    if (!g_key_get_string(file_meta, "vdagent-file-xfer", "name", file_name, sizeof(file_name)) ||
             !g_key_get_uint64(file_meta, "vdagent-file-xfer", "size", &file_size)) {
         vd_printf("file id %u meta parsing failed", start->id);
         return;
@@ -181,10 +181,12 @@ bool FileXfer::dispatch(VDAgentMessage* msg, VDAgentFileXferStatusMessage* statu
 //minimal parsers for GKeyFile, supporting only key=value with no spaces.
 #define G_KEY_MAX_LEN 256

-bool FileXfer::g_key_get_string(char* data, const char* group, const char* key, char* value)
+bool FileXfer::g_key_get_string(char* data, const char* group, const char* key, char* value,
+                                                  unsigned vsize)
 {
     char group_pfx[G_KEY_MAX_LEN], key_pfx[G_KEY_MAX_LEN];
-    char *group_pos, *key_pos, *next_group_pos;
+    char *group_pos, *key_pos, *next_group_pos, *start, *end;
+    unsigned len;

     snprintf(group_pfx, sizeof(group_pfx), "[%s]", group);
     if (!(group_pos = strstr((char*)data, group_pfx))) return false;
@@ -193,15 +195,26 @@ bool FileXfer::g_key_get_string(char* data, const char* group, const char* key,
     if (!(key_pos = strstr(group_pos, key_pfx))) return false;

     next_group_pos = strstr(group_pos + strlen(group_pfx), "[");
-    if (next_group_pos && key_pos > next_group_pos) return false; 
+    if (next_group_pos && key_pos > next_group_pos) return false;

-    return !!sscanf(key_pos + strlen(key_pfx), "%s\n", value);
+    start = key_pos + strlen(key_pfx);
+    end = strchr(start, '\n');
+    if (!end) return false;
+
+    len = end - start;
+    if (len >= vsize) return false;
+
+    memcpy(value, start, len);
+    value[len] = '\0';
+
+    return true;
 }

 bool FileXfer::g_key_get_uint64(char* data, const char* group, const char* key, uint64_t* value)
 {
     char str[G_KEY_MAX_LEN];

-    if (!g_key_get_string(data, group, key, str)) return false;
+    if (!g_key_get_string(data, group, key, str, sizeof(str)))
+        return false;
     return !!sscanf(str, "%" PRIu64, value);
 }
diff --git a/vdagent/file_xfer.h b/vdagent/file_xfer.h
index 649b296..b506f59 100644
--- a/vdagent/file_xfer.h
+++ b/vdagent/file_xfer.h
@@ -44,7 +44,8 @@ private:
     void handle_start(VDAgentFileXferStartMessage* start, VDAgentFileXferStatusMessage* status);
     bool handle_data(VDAgentFileXferDataMessage* data, VDAgentFileXferStatusMessage* status);
     void handle_status(VDAgentFileXferStatusMessage* status);
-    bool g_key_get_string(char* data, const char* group, const char* key, char* value);
+    bool g_key_get_string(char* data, const char* group, const char* key, char* value,
+                                        unsigned vsize);
     bool g_key_get_uint64(char* data, const char* group, const char* key, uint64_t* value);

 private:
-- 
1.8.3.1