[libvirt] [PATCH] Fix XML escaping problem in the XM driver

Daniel Veillard veillard at redhat.com
Thu Jun 19 15:44:37 UTC 2008


  Basically the XM driver when generating the XML config forgets to
escape things like filepaths allowing characters which are forbidden
as-is in XML instances to pass in the resulting config files which 
then are unusable.
  The patch also adds a new test checking the escaping is done
in a few critical places.

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/
-------------- next part --------------
Index: src/xm_internal.c
===================================================================
RCS file: /data/cvs/libxen/src/xm_internal.c,v
retrieving revision 1.82
diff -u -p -u -p -r1.82 xm_internal.c
--- src/xm_internal.c	10 Jun 2008 14:39:58 -0000	1.82
+++ src/xm_internal.c	19 Jun 2008 15:31:25 -0000
@@ -599,7 +599,7 @@ char *xenXMDomainFormatXML(virConnectPtr
         return (NULL);
 
     virBufferAddLit(&buf, "<domain type='xen'>\n");
-    virBufferVSprintf(&buf, "  <name>%s</name>\n", name);
+    virBufferEscapeString(&buf, "  <name>%s</name>\n", name);
     virUUIDFormat(uuid, uuidstr);
     virBufferVSprintf(&buf, "  <uuid>%s</uuid>\n", uuidstr);
 
@@ -612,7 +612,7 @@ char *xenXMDomainFormatXML(virConnectPtr
         virBufferAddLit(&buf, "  <os>\n");
         virBufferAddLit(&buf, "    <type>hvm</type>\n");
         if (xenXMConfigGetString(conf, "kernel", &str) == 0)
-            virBufferVSprintf(&buf, "    <loader>%s</loader>\n", str);
+            virBufferEscapeString(&buf, "    <loader>%s</loader>\n", str);
 
         if (xenXMConfigGetString(conf, "boot", &boot) < 0)
             boot = "c";
@@ -639,15 +639,15 @@ char *xenXMDomainFormatXML(virConnectPtr
     } else {
 
         if (xenXMConfigGetString(conf, "bootloader", &str) == 0)
-            virBufferVSprintf(&buf, "  <bootloader>%s</bootloader>\n", str);
+            virBufferEscapeString(&buf, "  <bootloader>%s</bootloader>\n", str);
         if (xenXMConfigGetString(conf, "bootargs", &str) == 0)
             virBufferEscapeString(&buf, "  <bootloader_args>%s</bootloader_args>\n", str);
         if (xenXMConfigGetString(conf, "kernel", &str) == 0) {
             virBufferAddLit(&buf, "  <os>\n");
             virBufferAddLit(&buf, "    <type>linux</type>\n");
-            virBufferVSprintf(&buf, "    <kernel>%s</kernel>\n", str);
+            virBufferEscapeString(&buf, "    <kernel>%s</kernel>\n", str);
             if (xenXMConfigGetString(conf, "ramdisk", &str) == 0)
-                virBufferVSprintf(&buf, "    <initrd>%s</initrd>\n", str);
+                virBufferEscapeString(&buf, "    <initrd>%s</initrd>\n", str);
             if (xenXMConfigGetString(conf, "extra", &str) == 0)
                 virBufferEscapeString(&buf, "    <cmdline>%s</cmdline>\n", str);
             virBufferAddLit(&buf, "  </os>\n");
@@ -714,7 +714,7 @@ char *xenXMDomainFormatXML(virConnectPtr
 
     if (hvm) {
         if (xenXMConfigGetString(conf, "device_model", &str) == 0)
-            virBufferVSprintf(&buf, "    <emulator>%s</emulator>\n", str);
+            virBufferEscapeString(&buf, "    <emulator>%s</emulator>\n", str);
     }
 
     list = virConfGetValue(conf, "disk");
@@ -816,9 +816,12 @@ char *xenXMDomainFormatXML(virConnectPtr
                 virBufferVSprintf(&buf, "      <driver name='%s' type='%s'/>\n", drvName, drvType);
             else
                 virBufferVSprintf(&buf, "      <driver name='%s'/>\n", drvName);
-            if (src[0])
-                virBufferVSprintf(&buf, "      <source %s='%s'/>\n", block ? "dev" : "file", src);
-            virBufferVSprintf(&buf, "      <target dev='%s' bus='%s'/>\n", dev, bus);
+            if (src[0]) {
+                virBufferVSprintf(&buf, "      <source %s=", block ? "dev" : "file");
+		virBufferEscapeString(&buf, "'%s'/>\n",  src);
+	    }
+            virBufferEscapeString(&buf, "      <target dev='%s'", dev);
+            virBufferVSprintf(&buf, " bus='%s'/>\n", bus);
             if (STREQ(head, "r") ||
                 STREQ(head, "ro"))
                 virBufferAddLit(&buf, "      <readonly/>\n");
@@ -836,7 +839,7 @@ char *xenXMDomainFormatXML(virConnectPtr
         if (xenXMConfigGetString(conf, "cdrom", &str) == 0) {
             virBufferAddLit(&buf, "    <disk type='file' device='cdrom'>\n");
             virBufferAddLit(&buf, "      <driver name='file'/>\n");
-            virBufferVSprintf(&buf, "      <source file='%s'/>\n", str);
+            virBufferEscapeString(&buf, "      <source file='%s'/>\n", str);
             virBufferAddLit(&buf, "      <target dev='hdc' bus='ide'/>\n");
             virBufferAddLit(&buf, "      <readonly/>\n");
             virBufferAddLit(&buf, "    </disk>\n");
@@ -924,7 +927,7 @@ char *xenXMDomainFormatXML(virConnectPtr
             if (type == 1 && bridge[0])
                 virBufferVSprintf(&buf, "      <source bridge='%s'/>\n", bridge);
             if (script[0])
-                virBufferVSprintf(&buf, "      <script path='%s'/>\n", script);
+                virBufferEscapeString(&buf, "      <script path='%s'/>\n", script);
             if (ip[0])
                 virBufferVSprintf(&buf, "      <ip address='%s'/>\n", ip);
             if (model[0])
@@ -1024,10 +1027,10 @@ char *xenXMDomainFormatXML(virConnectPtr
             virBufferVSprintf(&buf, " listen='%s'", vnclisten);
         }
         if (vncpasswd) {
-            virBufferVSprintf(&buf, " passwd='%s'", vncpasswd);
+            virBufferEscapeString(&buf, " passwd='%s'", vncpasswd);
         }
         if (keymap) {
-            virBufferVSprintf(&buf, " keymap='%s'", keymap);
+            virBufferEscapeString(&buf, " keymap='%s'", keymap);
         }
         virBufferAddLit(&buf, "/>\n");
     }
Index: tests/xmconfigtest.c
===================================================================
RCS file: /data/cvs/libxen/tests/xmconfigtest.c,v
retrieving revision 1.18
diff -u -p -u -p -r1.18 xmconfigtest.c
--- tests/xmconfigtest.c	29 May 2008 15:31:49 -0000	1.18
+++ tests/xmconfigtest.c	19 Jun 2008 15:31:25 -0000
@@ -223,6 +223,7 @@ mymain(int argc, char **argv)
 
     DO_TEST("fullvirt-sound", 2);
 
+    DO_TEST("escape-paths", 2);
     return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
 }
 
Index: tests/xmconfigdata/test-escape-paths.cfg
===================================================================
RCS file: tests/xmconfigdata/test-escape-paths.cfg
diff -N tests/xmconfigdata/test-escape-paths.cfg
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ tests/xmconfigdata/test-escape-paths.cfg	19 Jun 2008 15:32:59 -0000
@@ -0,0 +1,26 @@
+name = "XenGuest2&test"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+builder = "hvm"
+kernel = "/usr/lib/xen/boot/hvmloader&test"
+boot = "d"
+pae = 1
+acpi = 1
+apic = 1
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+device_model = "/usr/lib/xen/bin/qemu-dm&test"
+sdl = 0
+vnc = 1
+vncunused = 1
+vnclisten = "127.0.0.1"
+vncpasswd = "123poi"
+disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", "file:/root/boot.iso&test,hdc:cdrom,r" ]
+vif = [ "mac=00:16:3E:66:92:9C,bridge=xenbr1,type=ioemu" ]
+parallel = "none"
+serial = "none"
+soundhw = "sb16,es1370"
Index: tests/xmconfigdata/test-escape-paths.xml
===================================================================
RCS file: tests/xmconfigdata/test-escape-paths.xml
diff -N tests/xmconfigdata/test-escape-paths.xml
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ tests/xmconfigdata/test-escape-paths.xml	19 Jun 2008 15:32:59 -0000
@@ -0,0 +1,43 @@
+<domain type='xen'>
+  <name>XenGuest2&test</name>
+  <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+  <os>
+    <type>hvm</type>
+    <loader>/usr/lib/xen/boot/hvmloader&test</loader>
+    <boot dev='cdrom'/>
+  </os>
+  <currentMemory>403456</currentMemory>
+  <memory>592896</memory>
+  <vcpu>1</vcpu>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <features>
+    <pae/>
+    <acpi/>
+    <apic/>
+  </features>
+  <clock offset='utc'/>
+  <devices>
+    <emulator>/usr/lib/xen/bin/qemu-dm&test</emulator>
+    <disk type='block' device='disk'>
+      <driver name='phy'/>
+      <source dev='/dev/HostVG/XenGuest2'/>
+      <target dev='hda' bus='ide'/>
+    </disk>
+    <disk type='file' device='cdrom'>
+      <driver name='file'/>
+      <source file='/root/boot.iso&test'/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='00:16:3E:66:92:9C'/>
+      <source bridge='xenbr1'/>
+    </interface>
+    <input type='mouse' bus='ps2'/>
+    <graphics type='vnc' port='-1' listen='127.0.0.1' passwd='123poi'/>
+    <sound model='sb16'/>
+    <sound model='es1370'/>
+  </devices>
+</domain>


More information about the libvir-list mailing list