[PATCH 3/7] vbox: avoid problematic uses of sprintf

[Eric Blake <eblake@xxxxxxxxxx>]

* src/vbox/vbox_tmpl.c (vboxStartMachine, vboxAttachUSB): Use
virAsprintf instead.
---

This removes all use of sprintf in vbox.  The first 3 use virAsprintf
(DISPLAY may be arbitrarily long, and while we are unlikely to hit
9999 devices, it's better to be safe than to risk silent buffer
overflow); the remaining two are sized appropriately (actually, they
are sized too large, the real boundary size would be sizeof(int)*2+1
rather than 40); I felt better using snprintf rather than sprintf.

This doesn't address the fact that vbox doesn't really have very good
OOM handling (ie. it keeps on trying, although after the first OOM,
it will likely get another one); but that is an independent issue.

 src/vbox/vbox_tmpl.c |   39 +++++++++++++++++++++++++++------------
 1 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 3e8ff23..f50a12e 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -3161,7 +3161,6 @@ vboxStartMachine(virDomainPtr dom, int i, IMachine *machine, vboxIID *iid)
     PRUnichar *valueDisplayUtf16 = NULL;
     char      *valueDisplayUtf8  = NULL;
     IProgress *progress          = NULL;
-    char displayutf8[32]         = {0};
     PRUnichar *env               = NULL;
     PRUnichar *sessionType       = NULL;
     nsresult rc;
@@ -3241,8 +3240,13 @@ vboxStartMachine(virDomainPtr dom, int i, IMachine *machine, vboxIID *iid)

     if (guiPresent) {
         if (guiDisplay) {
-            sprintf(displayutf8, "DISPLAY=%.24s", guiDisplay);
-            VBOX_UTF8_TO_UTF16(displayutf8, &env);
+            char *displayutf8;
+            if (virAsprintf(&displayutf8, "DISPLAY=%s", guiDisplay) < 0)
+                virReportOOMError();
+            else {
+                VBOX_UTF8_TO_UTF16(displayutf8, &env);
+                VIR_FREE(displayutf8);
+            }
             VIR_FREE(guiDisplay);
         }

@@ -3251,8 +3255,13 @@ vboxStartMachine(virDomainPtr dom, int i, IMachine *machine, vboxIID *iid)

     if (sdlPresent) {
         if (sdlDisplay) {
-            sprintf(displayutf8, "DISPLAY=%.24s", sdlDisplay);
-            VBOX_UTF8_TO_UTF16(displayutf8, &env);
+            char *displayutf8;
+            if (virAsprintf(&displayutf8, "DISPLAY=%s", sdlDisplay) < 0)
+                virReportOOMError();
+            else {
+                VBOX_UTF8_TO_UTF16(displayutf8, &env);
+                VIR_FREE(displayutf8);
+            }
             VIR_FREE(sdlDisplay);
         }

@@ -4457,15 +4466,19 @@ vboxAttachUSB(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
                     if (def->hostdevs[i]->source.subsys.type ==
                         VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {

-                        char filtername[11]        = {0};
+                        char *filtername           = NULL;
                         PRUnichar *filternameUtf16 = NULL;
                         IUSBDeviceFilter *filter   = NULL;

-                        /* Assuming can't have more then 9999 devices so
-                         * restricting to %04d
+                        /* Zero pad for nice alignment when fewer than 9999
+                         * devices.
                          */
-                        sprintf(filtername, "filter%04d", i);
-                        VBOX_UTF8_TO_UTF16(filtername, &filternameUtf16);
+                        if (virAsprintf(&filtername, "filter%04d", i) < 0) {
+                            virReportOOMError();
+                        } else {
+                            VBOX_UTF8_TO_UTF16(filtername, &filternameUtf16);
+                            VIR_FREE(filtername);
+                        }

                         USBController->vtbl->CreateDeviceFilter(USBController,
                                                                 filternameUtf16,
@@ -4482,13 +4495,15 @@ vboxAttachUSB(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
                             char productId[40]        = {0};

                             if (def->hostdevs[i]->source.subsys.u.usb.vendor) {
-                                sprintf(vendorId, "%x", def->hostdevs[i]->source.subsys.u.usb.vendor);
+                                snprintf(vendorId, sizeof(vendorId), "%x",
+                                         def->hostdevs[i]->source.subsys.u.usb.vendor);
                                 VBOX_UTF8_TO_UTF16(vendorId, &vendorIdUtf16);
                                 filter->vtbl->SetVendorId(filter, vendorIdUtf16);
                                 VBOX_UTF16_FREE(vendorIdUtf16);
                             }
                             if (def->hostdevs[i]->source.subsys.u.usb.product) {
-                                sprintf(productId, "%x", def->hostdevs[i]->source.subsys.u.usb.product);
+                                snprintf(productId, sizeof(productId), "%x",
+                                         def->hostdevs[i]->source.subsys.u.usb.product);
                                 VBOX_UTF8_TO_UTF16(productId, &productIdUtf16);
                                 filter->vtbl->SetProductId(filter,
                                                            productIdUtf16);
-- 
1.7.2.2

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