i#1542: handle double quotes in client options

Augments drconfiglib to pick a -client_lib delimiter to avoid quote
character conflicts.

Adds DR_CONFIG_OPTIONS_INVALID, with drconfiglib returning it when it finds
; or can't find a delimiter (== client path + options together contain all
3 quote types).

Documents these limitations in intro.dox and dr_config.h.

Fixes issue 1542

SVN-Revision: 2835

api/docs/intro.dox

@@ -505,31 +505,35 @@ All of the earlier examples did not need to pass any arguments to the client.
 When using the -c argument to set the client, all arguments between the client
 path and the double dash are passed to the client.  When using the -client
 argument to drrun, the third argument following -client is passed through to
-the client.  For example, all these invocations of drrun pass "-client_op1
--client_op2" to the client:
+the client.  For example, all these invocations of drrun pass '-op1
+-op2 "value with spaces"' to the client:
 
 \code
-bin32/drrun.exe -c libmyclient.dll -client_op1 -client_op2 -- calc
-bin32/drrun.exe -client myclient.dll 0 "-client_op1 -client_op2" -- calc
+bin32/drrun.exe -c libmyclient.dll -op1 -op2 \"value with spaces\" -- calc
+bin32/drrun.exe -client myclient.dll 0 '-op1 -op2 "value with spaces"' -- calc
 \endcode
 
 On Linux:
 
 \code
-bin32/drrun -c libmyclient.so client_op1 -client_op2 -- ls
-bin32/drrun -client libmyclient.so 0 "-client_op1 -client_op2" -- ls
+bin32/drrun -c libmyclient.so -op1 -op2 \"value with spaces\" -- ls
+bin32/drrun -client libmyclient.so 0 '-op1 -op2 "value with spaces"' -- ls
 \endcode
 
 When using a two-step model, the options are passed to \p drconfig:
 
 \code
-bin32/drconfig.exe -reg calc.exe -c myclient.dll -client_op1 -client_op2
-bin32/drconfig.exe -reg calc.exe -client myclient.dll 0 "-client_op1 -client_op2"
+bin32/drconfig.exe -reg calc.exe -c myclient.dll -op1 -op2 \"value with spaces\"
+bin32/drconfig.exe -reg calc.exe -client myclient.dll 0 '-op1 -op2 "value with spaces"'
 \endcode
 
 The client can call dr_get_options() to retrieve the option string passed
 to it.
 
+Client options are not allowed to contain semicolons.  Additionally, the
+client option string combined with the path to the client library cannot
+contain all three quote characters (', ", `) simultaneously.
+
 \section multi_client Multiple Clients
 
 DynamoRIO does support multiple clients.  It is each client's
@@ -1711,7 +1715,7 @@ Options available only in the debug build of DynamoRIO:
 
  - \b -ignore_assert_list \b '*': \anchor op_ignore_assert
     Ignores all DynamoRIO asserts of the form "<file>:1234". * may be
-    replaced by a ; separated lists of individual asserts to ignore
+    replaced by a ; separated list of individual asserts to ignore
     "foo.c:333;bar.c:12".
 
 ***************************************************************************

core/lib/dr_config.h

@@ -130,6 +130,9 @@ typedef enum {
     /** Nudge operation failed because the specified process id does not exist. */
     DR_NUDGE_PID_NOT_FOUND,
 
+    /** Client options contain invalid characters (';' or all 3 quotes). */
+    DR_CONFIG_OPTIONS_INVALID,
+
 } dr_config_status_t;
 
 /** Allow targeting both 32-bit and native 64-bit processes separately. */
@@ -612,12 +615,16 @@ DR_EXPORT
  * \param[in]   client_path     A NULL-terminated string specifying the full path
  *                              to a valid client library.  The string length
  *                              cannot exceed MAX_PATH.  The client path may not
- *                              include any semicolons.
+ *                              include any semicolons and when combined with
+ *                              \p client_options may not include all
+ *                              three quote characters (', ", `) simultaneously.
  *
  * \param[in]   client_options  A NULL-terminated string specifying options that
  *                              are available to the client via dr_get_options().
  *                              The string length cannot exceed #DR_MAX_OPTIONS_LENGTH.
- *                              The client options may not include any semicolons.
+ *                              The client options may not include any semicolons
+ *                              and when combined with \p client_path may not include
+ *                              all three quote characters (', ", `) simultaneously.
  *
  * \return      A dr_config_status_t code indicating the result of registration.
  */

libutil/dr_config.c

@@ -806,7 +806,7 @@ read_options(opt_info_t *opt_info, IF_REG_ELSE(ConfigGroup *proc_policy, FILE *f
 /* Write the options stored in an opt_info_t to 'wbuf' in the form expected
  * by the DYNAMORIO_OPTIONS registry entry.
  */
-static void
+static dr_config_status_t
 write_options(opt_info_t *opt_info, TCHAR *wbuf)
 {
     size_t i;
@@ -874,14 +874,30 @@ write_options(opt_info_t *opt_info, TCHAR *wbuf)
     /* client lib options */
     for (i=0; i<opt_info->num_clients; i++) {
         client_opt_t *client_opts = opt_info->client_opts[i];
+        /* i#1542: pick a delimiter that avoids conflicts w/ the client strings */
+        char delim = '\"';
+        if (strchr(client_opts->path, delim) || strchr(client_opts->opts, delim)) {
+             delim = '\'';
+             if (strchr(client_opts->path, delim) || strchr(client_opts->opts, delim)) {
+                 delim = '`';
+                 if (strchr(client_opts->path, delim) ||
+                     strchr(client_opts->opts, delim)) {
+                     return DR_CONFIG_OPTIONS_INVALID;
+                 }
+             }
+        }
+        /* no ; allowed */
+        if (strchr(client_opts->path, ';') || strchr(client_opts->opts, ';'))
+            return DR_CONFIG_OPTIONS_INVALID;
         len = _sntprintf(wbuf + sofar, bufsz - sofar,
-                         _TEXT(" -client_lib \"%s;%x;%s\""),
-                         client_opts->path, client_opts->id, client_opts->opts);
+                         _TEXT(" -client_lib %c%s;%x;%s%c"), delim,
+                         client_opts->path, client_opts->id, client_opts->opts, delim);
         if (len >= 0 && len <= bufsz - sofar)
             sofar += len;
     }
 
     wbuf[DR_MAX_OPTIONS_LENGTH-1] = L'\0';
+    return DR_SUCCESS;
 }
 
 #ifdef PARAMS_IN_REGISTRY
@@ -1095,7 +1111,9 @@ dr_register_process(const char *process_name,
     /* set the options string last for faster updating w/ config files */
     opt_info.mode = dr_mode;
     add_extra_option_char(&opt_info, dr_options);
-    write_options(&opt_info, wbuf);
+    status = write_options(&opt_info, wbuf);
+    if (status != DR_SUCCESS)
+        return status;
     status = write_config_param(IF_REG_ELSE(proc_policy, f),
                                 PARAM_STR(DYNAMORIO_VAR_OPTIONS), wbuf);
     free_opt_info(&opt_info);
@@ -1668,7 +1686,9 @@ dr_register_client(const char *process_name,
     }
 
     /* write the registry */
-    write_options(&opt_info, new_opts);
+    status = write_options(&opt_info, new_opts);
+    if (status != DR_SUCCESS)
+        goto exit;
 #ifndef PARAMS_IN_REGISTRY
     /* shift rest of file up, overwriting old value, so we can append new value */
     read_config_ex(f, DYNAMORIO_VAR_OPTIONS, NULL, 0, true);

tools/drdeploy.c

@@ -586,6 +586,10 @@ bool register_client(const char *process_name,
         if (status == DR_CONFIG_STRING_TOO_LONG) {
             error("client %s registration failed: option string too long: \"%s\"",
                   path == NULL ? "<null>" : path, options);
+        } else if (status == DR_CONFIG_OPTIONS_INVALID) {
+            error("client %s registration failed: options cannot contain ';' or all "
+                  "3 quote types: %s",
+                  path == NULL ? "<null>" : path, options);
         } else {
             error("client %s registration failed with error code %d",
                   path == NULL ? "<null>" : path, status);