[PATCH] properly check buffer size in virDomainXMLDevID

[Hugh Brock <hbrock@xxxxxxxxxx>]

As promised, a patch to protect the 80-character "device id" buffer from 
overflow by the unbounded "device=" XML attribute. Before, a large 
"device" attribute gave a stack overflow error; now it merely results in 
an obscure (but non-fatal) xend error like so:


libvir: Xen Daemon error : POST operation failed: (xend.err "invalid 
literal for int() with base 10: 
'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'")

(the long string of "x"es was my way of overflowing the buffer).

Please ACK...

--Hugh

--
Red Hat Virtualization Group http://redhat.com/virtualization
Hugh Brock           | virt-manager http://virt-manager.org
hbrock@xxxxxxxxxx    | virtualization library http://libvirt.org

diff -ruN libvirt.orig/src/xend_internal.c libvirt/src/xend_internal.c
--- libvirt.orig/src/xend_internal.c	2007-09-11 09:11:25.000000000 -0400
+++ libvirt/src/xend_internal.c	2007-09-11 09:07:58.000000000 -0400
@@ -3117,7 +3117,7 @@
         *(conf + strlen(conf) -1) = 0; /* suppress final ) */
     }
     else conf = sexpr;
-    if (virDomainXMLDevID(domain, xml, class, ref)) {
+    if (virDomainXMLDevID(domain, xml, class, ref, sizeof(ref))) {
         /* device doesn't exist, define it */
         ret = xend_op(domain->conn, domain->name, "op", "device_create",
                       "config", conf, NULL);
@@ -3150,7 +3150,7 @@
 	             __FUNCTION__);
         return (-1);
     }
-    if (virDomainXMLDevID(domain, xml, class, ref))
+    if (virDomainXMLDevID(domain, xml, class, ref, sizeof(ref)))
         return (-1);
     return(xend_op(domain->conn, domain->name, "op", "device_destroy",
         "type", class, "dev", ref, NULL));

diff -ruN libvirt.orig/src/xml.c libvirt/src/xml.c
--- libvirt.orig/src/xml.c	2007-09-06 11:21:20.000000000 -0400
+++ libvirt/src/xml.c	2007-09-11 08:58:46.000000000 -0400
@@ -1385,7 +1385,7 @@
  * Returns 0 in case of success, -1 in case of failure.
  */
 int
-virDomainXMLDevID(virDomainPtr domain, char *xmldesc, char *class, char *ref)
+virDomainXMLDevID(virDomainPtr domain, char *xmldesc, char *class, char *ref, int ref_len)
 {
     xmlDocPtr xml = NULL;
     xmlNodePtr node, cur;
@@ -1413,7 +1413,8 @@
             attr = xmlGetProp(cur, BAD_CAST "dev");
             if (attr == NULL)
                 goto error;
-            strcpy(ref, (char *)attr);
+            strncpy(ref, (char *)attr, ref_len);
+               ref[ref_len -1] = '\0';
             goto cleanup;
         }
     }
@@ -1430,8 +1431,9 @@
             xref = xenStoreDomainGetNetworkID(domain->conn, domain->id,
                                               (char *) attr);
             if (xref != NULL) {
-                strcpy(ref, xref);
+                strncpy(ref, xref, ref_len);
                 free(xref);
+                ref[ref_len - 1] = '\0';
                 goto cleanup;
             }
 #else /* without xen */
diff -ruN libvirt.orig/src/xml.h libvirt/src/xml.h
--- libvirt.orig/src/xml.h	2007-08-16 11:19:32.000000000 -0400
+++ libvirt/src/xml.h	2007-09-06 14:58:50.000000000 -0400
@@ -33,7 +33,7 @@
 
 char *virDomainParseXMLDesc(virConnectPtr conn, const char *xmldesc, char **name, int xendConfigVersion);
 char *virParseXMLDevice(virConnectPtr conn, char *xmldesc, int hvm, int xendConfigVersion);
-int virDomainXMLDevID(virDomainPtr domain, char *xmldesc, char *class, char *ref);
+  int virDomainXMLDevID(virDomainPtr domain, char *xmldesc, char *class, char *ref, int ref_len);
 
 #ifdef __cplusplus
 }

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list