Improve clipboard .metadata parsing

Better parsing and error handling for
`/run/qubes/qubes-clipboard.bin.metadata`

gui-daemon/xside.c

@@ -859,33 +859,56 @@ static void save_clipboard_metadata(struct clipboard_metadata *metadata) {
     umask(old_umask);
 }
 
-static bool load_clipboard_metadata(struct clipboard_metadata *metadata) {
+static bool load_clipboard_metadata(struct clipboard_metadata *metadata, bool logging) {
     FILE *file;
     char line[256];
-    char key[256];
-    char value[256];
+    char key[256] = {0};
+    char value[256] = {0};
 
     file = fopen(QUBES_CLIPBOARD_FILENAME ".metadata", "r");
     if (!file) {
-        perror("Can not open " QUBES_CLIPBOARD_FILENAME ".metadata file");
+        if (logging)
+            perror("Can not open " QUBES_CLIPBOARD_FILENAME ".metadata file");
         return false;
     }
     // Load JSON format
     while (fgets(line, sizeof(line), file) != NULL) {
+        if (strlen(line) == 0) continue;
+        if (line[strlen(line) - 1] == '\n') line[strlen(line) - 1] = 0;
         if (strcmp(line, "") == 0) continue;
         if (strcmp(line, "{") == 0) continue;
         if (strcmp(line, "}") == 0) continue;
-        if (! sscanf(line, "\"%[A-Za-z0-9]\":%[\"A-Za-z0-9]s,", key, value)) {
-            fprintf (stderr, "Failed to parse metadata line: %s\n", line);
+        if (sscanf(line, "\"%[A-Za-z0-9_-]\":%[\"A-Za-z0-9_-]s", key, value) != 2) {
+            if (logging) {
+                fprintf (stderr, "Failed to parse clipboard metadata line: %s\n", line);
+            }
             return false;
         }
         if (strcmp(key, "vmname") == 0) {
             /* value should be less than allowed maximum vmlenght + 2
              * considering the quotation marks */
-            if (strlen(value) >= 32 + 2) return false;
-            if (strlen(value) < 2) return false;
+            if ((strlen(value) >= 32 + 2) ||
+                (strlen(value) < 2) ||
+                (value[0] != '"') ||
+                (value[strlen(value) - 1] != '"')) {
+                if (logging)
+                    fprintf (stderr, "Clipboard vmname value should be less than 32 characters and between double-quotes: %s\n", line);
+                return false;
+            }
             strncpy (metadata->vmname, value + 1, strlen(value) - 2);
-        } else if (strcmp(key, "xevent_timestamp") == 0) {
+            continue;
+        }
+
+        /* From this point on, all recognized values are essentially integers */
+        long unsigned int dummy;
+        if (sscanf(value, "%lu", &dummy) != 1) {
+            if (logging) {
+                fprintf (stderr, "Failed to parse clipboard metadata: key=%s, value=%s\n", key, value);
+            }
+            return false;
+        }
+
+        if (strcmp(key, "xevent_timestamp") == 0) {
             sscanf(value, "%lu", &metadata->xevent_timestamp);
         } else if (strcmp(key, "successful") == 0) {
             sscanf(value, "%d", (int *)&metadata->successful);
@@ -906,13 +929,13 @@ static bool load_clipboard_metadata(struct clipboard_metadata *metadata) {
         } else if (strcmp(key, "buffer_size") == 0) {
             sscanf(value, "%d", &metadata->buffer_size);
         } else if (strcmp(key, "protocol_version_vmside") == 0) {
-            sscanf(value, "%x", &metadata->protocol_version_vmside);
+            sscanf(value, "%d", &metadata->protocol_version_vmside);
         } else if (strcmp(key, "protocol_version_xside") == 0) {
-            sscanf(value, "%x", &metadata->protocol_version_xside);
+            sscanf(value, "%d", &metadata->protocol_version_xside);
         }
     }
     fclose(file);
-    return metadata->cleared;
+    return true;
 }
 
 /* caller must take inter_appviewer_lock first */
@@ -926,10 +949,29 @@ static void clear_clipboard(struct clipboard_metadata *metadata) {
     save_clipboard_source_vmname("");
 }
 
-static Time get_clipboard_xevent_timestamp() {
+/* caller must take inter_appviewer_lock first */
+static Time get_clipboard_xevent_timestamp(bool logging) {
     struct clipboard_metadata metadata = {0};
-    load_clipboard_metadata(&metadata);
-    return metadata.xevent_timestamp;
+    FILE *file;
+
+    /* do not do a detailed logging for metadata parsing. handle non-existent
+     * .metadata file independently here */
+    file = fopen(QUBES_CLIPBOARD_FILENAME ".metadata", "r");
+    if (!file) {
+        if (logging)
+            perror("Can not get xevent timestamp from non-existent " QUBES_CLIPBOARD_FILENAME ".metadata");
+        return 0;
+    } else {
+       fclose(file);
+    }
+
+    if (!load_clipboard_metadata(&metadata, logging)) {
+        if (logging)
+            perror("Can not get xevent timestamp from " QUBES_CLIPBOARD_FILENAME ".metadata");
+        return 0;
+    } else {
+        return metadata.xevent_timestamp;
+    }
 }
 
 /* fetch clippboard content from file */
@@ -1193,8 +1235,15 @@ static void handle_clipboard_data(Ghandles * g, unsigned int untrusted_len)
         clear_clipboard(&metadata);
         return;
     }
+    if (metadata.sent_size == 0) {
+        /* source vm clipboard is empty. clear .data and update .metadata */
+        free(untrusted_data);
+        metadata.successful = true;
+        clear_clipboard(&metadata);
+        return;
+    }
     inter_appviewer_lock(g, 1);
-    clipboard_file_xevent_time = get_clipboard_xevent_timestamp();
+    clipboard_file_xevent_time = get_clipboard_xevent_timestamp(g->log_level > 0);
     /* X11 time is just 32-bit miliseconds counter, which make it wrap every
      * ~50 days - something that is realistic. Handle that wrapping too. */
     if (clipboard_file_xevent_time - g->clipboard_xevent_time < (1UL<<31)) {
@@ -1469,7 +1518,7 @@ static int is_special_keypress(Ghandles * g, const XKeyEvent * ev, XID remote_wi
         if (ev->type != KeyPress)
             return 1;
         inter_appviewer_lock(g, 1);
-        clipboard_file_xevent_time = get_clipboard_xevent_timestamp();
+        clipboard_file_xevent_time = get_clipboard_xevent_timestamp(g->log_level > 0);
         /* X11 time is just 32-bit miliseconds counter, which make it wrap every
          * ~50 days - something that is realistic. Handle that wrapping too. */
         if (clipboard_file_xevent_time - ev->time < (1UL<<31)) {