The root directory can be provided by user (or a temporary one is
generated) and is always formatted into connection URI for both
secret driver and QEMU driver, like this:

  qemu:///embed?root=$root

But if it so happens that there is an URI unfriendly character in
root directory or path to it (say a space) then invalid URI is
formatted which results in unexpected results. We can trust
g_dir_make_tmp() to generate valid URI but we can't trust user.
Escape user provided root directory. Always.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1920400
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_shim.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_shim.c b/src/qemu/qemu_shim.c
index 18bdc99256..c10598df4b 100644
--- a/src/qemu/qemu_shim.c
+++ b/src/qemu/qemu_shim.c
@@ -140,7 +140,8 @@ int main(int argc, char **argv)
     g_autofree char *xml = NULL;
     g_autofree char *uri = NULL;
     g_autofree char *suri = NULL;
-    char *root = NULL;
+    const char *root = NULL;
+    g_autofree char *escaped = NULL;
     bool tmproot = false;
     int ret = 1;
     g_autoptr(GError) error = NULL;
@@ -216,6 +217,8 @@ int main(int argc, char **argv)
         }
     }
 
+    escaped = g_uri_escape_string(root, NULL, true);
+
     virFileActivateDirOverrideForProg(argv[0]);
 
     if (verbose)
@@ -242,7 +245,7 @@ int main(int argc, char **argv)
     eventLoopThread = g_thread_new("event-loop", qemuShimEventLoop, NULL);
 
     if (secrets && *secrets) {
-        suri = g_strdup_printf("secret:///embed?root=%s", root);
+        suri = g_strdup_printf("secret:///embed?root=%s", escaped);
 
         if (verbose)
             g_printerr("%s: %lld: opening %s\n",
@@ -303,7 +306,7 @@ int main(int argc, char **argv)
         }
     }
 
-    uri = g_strdup_printf("qemu:///embed?root=%s", root);
+    uri = g_strdup_printf("qemu:///embed?root=%s", escaped);
 
     if (verbose)
         g_printerr("%s: %lld: opening %s\n",
-- 
2.26.2

On Fri, Feb 12, 2021 at 05:42:19PM +0100, Michal Privoznik wrote:
> The root directory can be provided by user (or a temporary one is
> generated) and is always formatted into connection URI for both
> secret driver and QEMU driver, like this:
> 
>   qemu:///embed?root=$root
> 
> But if it so happens that there is an URI unfriendly character in
> root directory or path to it (say a space) then invalid URI is
> formatted which results in unexpected results. We can trust
> g_dir_make_tmp() to generate valid URI but we can't trust user.
> Escape user provided root directory. Always.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1920400
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_shim.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|