qrexec-client: Better validation of arguments

No change in behavior on valid inputs.

Author: DemiMarie

daemon/qrexec-client.c

@@ -19,6 +19,7 @@
  *
  */
 
+#include <limits.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <stdio.h>
@@ -415,44 +416,47 @@ _Noreturn static void usage(const char *const name)
             "  -w timeout - override default connection timeout of 5s (set 0 for no timeout)\n"
             "  -k - kill the domain right before exiting\n"
             "  --socket-dir=PATH -  directory for qrexec socket, default: %s\n",
-            name, QREXEC_DAEMON_SOCKET_DIR);
+            name ? name : "qrexec-client", QREXEC_DAEMON_SOCKET_DIR);
     exit(1);
 }
 
+static int parse_int(const char *str, const char *msg) {
+    long value;
+    char *end = (char *)str;
+
+    if (str[0] < '0' || str[0] > '9')
+        errx(1, "%s '%s' does not start with an ASCII digit", msg, str);
+    errno = 0;
+    value = strtol(str, &end, 0);
+    if (*end != '\0')
+        errx(1, "trailing junk '%s' after %s", end, msg);
+    if (errno == 0 && (value < 0 || value > INT_MAX))
+        errno = ERANGE;
+    if (errno)
+        err(1, "invalid %s '%s': strtol", msg, str);
+    return (int)value;
+}
+
 static void parse_connect(char *str, char **request_id,
         char **src_domain_name, int *src_domain_id)
 {
-    int i=0;
-    char *token = NULL;
-    char *separators = ",";
-
-    token = strtok(str, separators);
-    while (token)
-    {
-        switch (i)
-        {
-            case 0:
-                *request_id = token;
-                if (strlen(*request_id) >= sizeof(struct service_params)) {
-                    fprintf(stderr, "Invalid -c parameter (request_id too long, max %lu)\n",
-                            sizeof(struct service_params)-1);
-                    exit(1);
-                }
-                break;
-            case 1:
-                *src_domain_name = token;
-                break;
-            case 2:
-                *src_domain_id = atoi(token);
-                break;
-            default:
-                goto bad_c_param;
-        }
-        token = strtok(NULL, separators);
-        i++;
-    }
-    if (i == 3)
-        return;
+    char *token;
+
+    token = strchr(str, ',');
+    if (token == NULL)
+        goto bad_c_param;
+    if ((size_t)(token - str) >= sizeof(struct service_params))
+        errx(1, "Invalid -c parameter (request_id too long, max %zu)\n",
+             sizeof(struct service_params)-1);
+    *token = 0;
+    *request_id = str;
+    *src_domain_name = token + 1;
+    token = strchr(*src_domain_name, ',');
+    if (token == NULL)
+        goto bad_c_param;
+    *token = 0;
+    *src_domain_id = parse_int(token + 1, "source domain ID");
+    return;
 bad_c_param:
     fprintf(stderr, "Invalid -c parameter (should be: \"-c request_id,src_domain_name,src_domain_id\")\n");
     exit(1);
@@ -547,6 +551,11 @@ int main(int argc, char **argv)
     int prepare_ret;
     bool kill = false;
 
+    if (argc < 3) {
+        // certainly too few arguments
+        usage(argv[0]);
+    }
+
     setup_logging("qrexec-client");
 
     while ((opt = getopt_long(argc, argv, "hd:l:eEc:tTw:Wk", longopts, NULL)) != -1) {
@@ -564,6 +573,10 @@ int main(int argc, char **argv)
                 exit_with_code = 0;
                 break;
             case 'c':
+                if (request_id != NULL) {
+                    warnx("ERROR: -c passed more than once");
+                    usage(argv[0]);
+                }
                 parse_connect(optarg, &request_id, &src_domain_name, &src_domain_id);
                 is_service = 1;
                 break;
@@ -574,7 +587,7 @@ int main(int argc, char **argv)
                 replace_chars_stderr = 1;
                 break;
             case 'w':
-                connection_timeout = atoi(optarg);
+                connection_timeout = parse_int(optarg, "connection timeout");
                 break;
             case 'W':
                 wait_connection_end = 1;