[libvirt] [PATCHv2 2/2] openvz: avoid potential buffer overflow

[Eric Blake]

* src/openvz/openvz_conf.c (openvzLoadDomains): Replace unsafe
sscanf with safe direct parsing.
(openvzGetVEID): Avoid lost integer overflow detection.
(openvzAssignUUIDs): Likewise, and detect readdir failure.
---

v2: new patch; plugs a potential security hole, since
*scanf("%s",fixed_width_buffer) is exploitable, but the
exploit could only happen if /usr/sbin/vzlist is compromised.

 src/openvz/openvz_conf.c |   39 +++++++++++++++++++++++++--------------
 1 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 800c1f4..2d1c41a 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -435,7 +435,7 @@ openvzFreeDriver(struct openvz_driver *driver)

 int openvzLoadDomains(struct openvz_driver *driver) {
     int veid, ret;
-    char status[16];
+    char *status;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
     virDomainObjPtr dom = NULL;
     char temp[50];
@@ -451,13 +451,17 @@ int openvzLoadDomains(struct openvz_driver *driver) {
     if (virCommandRun(cmd, NULL) < 0)
         goto cleanup;

-    line = *outbuf ? outbuf : NULL;
-    while (line) {
-        if (sscanf(line, "%d %s\n", &veid, status) != 2) {
+    line = outbuf;
+    while (line[0] != '\0') {
+        if (virStrToLong_i(line, &status, 10, &veid) < 0 ||
+            *status++ != ' ' ||
+            (line = strchr(status, '\n')) == NULL) {
             openvzError(VIR_ERR_INTERNAL_ERROR, "%s",
                         _("Failed to parse vzlist output"));
             goto cleanup;
         }
+        *line++ = '\0';

         if (VIR_ALLOC(dom) < 0)
             goto no_memory;
@@ -527,10 +531,6 @@ int openvzLoadDomains(struct openvz_driver *driver) {

         virDomainObjUnlock(dom);
         dom = NULL;
-
-        line = strchr(line, '\n');
-        if (line)
-            line++;
     }

     virCommandFree(cmd);
@@ -953,8 +953,9 @@ static int openvzAssignUUIDs(void)
     DIR *dp;
     struct dirent *dent;
     char *conf_dir;
-    int vpsid, res;
-    char ext[8];
+    int vpsid;
+    char *ext;
+    int ret = 0;

     conf_dir = openvzLocateConfDir();
     if (conf_dir == NULL)
@@ -966,16 +967,25 @@ static int openvzAssignUUIDs(void)
         return 0;
     }

+    errno = 0;
     while ((dent = readdir(dp))) {
-        res = sscanf(dent->d_name, "%d.%5s", &vpsid, ext);
-        if (!(res == 2 && STREQ(ext, "conf")))
+        if (virStrToLong_i(dent->d_name, &ext, 10, &vpsid) < 0 ||
+            *ext++ != '.' ||
+            STRNEQ(ext, "conf"))
             continue;
         if (vpsid > 0) /* '0.conf' belongs to the host, ignore it */
             openvzSetUUID(vpsid);
+        errno = 0;
+    }
+    if (errno) {
+        openvzError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("Failed to scan configuration directory"));
+        ret = -1;
     }
+
     closedir(dp);
     VIR_FREE(conf_dir);
-    return 0;
+    return ret;
 }


@@ -987,6 +997,7 @@ static int openvzAssignUUIDs(void)
 int openvzGetVEID(const char *name) {
     virCommandPtr cmd;
     char *outbuf;
+    char *temp;
     int veid;
     bool ok;

@@ -999,7 +1010,7 @@ int openvzGetVEID(const char *name) {
     }

     virCommandFree(cmd);
-    ok = sscanf(outbuf, "%d\n", &veid) == 1;
+    ok = virStrToLong_i(outbuf, &temp, 10, &veid) == 0 && *temp == '\n';
     VIR_FREE(outbuf);

     if (ok && veid >= 0)
-- 
1.7.3.2