GUACAMOLE-1921: Process argv handler even when the connection is read-only.

Author: necouchman

src/protocols/vnc/argv.h

@@ -28,6 +28,12 @@
 /**
  * Handles a received argument value from a Guacamole "argv" instruction,
  * updating the given connection parameter.
+ *
+ * As noted in the user.c file, care should be taken when updating this
+ * callback to make sure that arguments are handled correctly when
+ * a connection is marked as read-only, and to make sure that any
+ * usage of this callback for non-owner users of a connection does
+ * not have unintended security implications.
  */
 guac_argv_callback guac_vnc_argv_callback;
 

src/protocols/vnc/user.c

@@ -85,10 +85,6 @@ int guac_vnc_user_join_handler(guac_user* user, int argc, char** argv) {
         if (!settings->disable_paste)
             user->clipboard_handler = guac_vnc_clipboard_handler;
 
-        /* Updates to connection parameters if we own the connection */
-        if (user->owner)
-            user->argv_handler = guac_argv_handler;
-
 #ifdef ENABLE_COMMON_SSH
         /* Set generic (non-filesystem) file upload handler */
         if (settings->enable_sftp && !settings->sftp_disable_upload)
@@ -97,6 +93,26 @@ int guac_vnc_user_join_handler(guac_user* user, int argc, char** argv) {
 
     }
 
+    /**
+     * Update connection parameters if we own the connection. 
+     *
+     * Note that the argv handler is called *regardless* of whether
+     * or not the connection is read-only, as this allows authentication
+     * to be prompted and processed even if the owner cannot send
+     * input to the remote session. In the future, if other argv handling
+     * is added to the VNC protocol, checks may need to be done within
+     * the argv handler to verify that read-only connections remain
+     * read-only.
+     *
+     * Also, this is only handled for the owner - if the argv handler
+     * is expanded to include non-owner users in the future, special
+     * care will need to be taken to make sure that the arguments
+     * processed by the handler do not have unintended security
+     * implications for non-owner users.
+     */
+    if (user->owner)
+        user->argv_handler = guac_argv_handler;
+
     return 0;
 
 }