From: Eduardo Otubo <otubo@linux.vnet.ibm.com>
Date: Thu, 15 Jul 2010 22:37:48 +0000 (-0300)
Subject: phyp: refactor phypListDomainsGeneric to eliminate buffer overflow
X-Git-Tag: v0.8.4~119
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6324949fb11ce0148f34410f0ebf8f1a69ad82da;p=thirdparty%2Flibvirt.git

phyp: refactor phypListDomainsGeneric to eliminate buffer overflow

src/phyp/phyp_driver.c:phypListDomainsGeneric was crashing due to a buffer
overflow if any line returned from virRun wasn't <=10 characters.

Since virStrToLong_i recognizes any non-numeric as a terminator (not
just NULL), there actually is no need to copy the number into a
separate string anyway, so this patch eliminates that copy, the fixed
length buffer, and therefore the potential to overflow.

This change also provided the oppurtunity to eliminate the character
counting loop, instead using the return from virStrToLong_i to point
past the end of the number, then simply skip the \n to get to the
next.
---

diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 7143933d81..aa4722ed3d 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -380,12 +380,10 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids,
     int system_type = phyp_driver->system_type;
     char *managed_system = phyp_driver->managed_system;
     int exit_status = 0;
-    int got = 0;
-    char *char_ptr;
-    unsigned int i = 0, j = 0;
-    char id_c[10];
+    int got = -1;
     char *cmd = NULL;
     char *ret = NULL;
+    char *line, *next_line;
     const char *state;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
 
@@ -394,8 +392,6 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids,
     else
         state = " ";
 
-    memset(id_c, 0, 10);
-
     virBufferAddLit(&buf, "lssyscfg -r lpar");
     if (system_type == HMC)
         virBufferVSprintf(&buf, " -m %s", managed_system);
@@ -410,37 +406,28 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids,
 
     ret = phypExec(session, cmd, &exit_status, conn);
 
-    /* I need to parse the textual return in order to get the ret */
     if (exit_status < 0 || ret == NULL)
         goto err;
-    else {
-        while (got < nids) {
-            if (ret[i] == '\0')
-                break;
-            else if (ret[i] == '\n') {
-                if (virStrToLong_i(id_c, &char_ptr, 10, &ids[got]) == -1) {
-                    VIR_ERROR(_("Cannot parse number from '%s'"), id_c);
-                    goto err;
-                }
-                memset(id_c, 0, 10);
-                j = 0;
-                got++;
-            } else {
-                id_c[j] = ret[i];
-                j++;
-            }
-            i++;
+
+    /* I need to parse the textual return in order to get the ids */
+    line = ret;
+    got = 0;
+    while (*line && got < nids) {
+        if (virStrToLong_i(line, &next_line, 10, &ids[got]) == -1) {
+            VIR_ERROR(_("Cannot parse number from '%s'"), line);
+            got = -1;
+            goto err;
         }
+        got++;
+        line = next_line;
+        while (*line == '\n')
+            line++; /* skip \n */
     }
 
-    VIR_FREE(cmd);
-    VIR_FREE(ret);
-    return got;
-
   err:
     VIR_FREE(cmd);
     VIR_FREE(ret);
-    return -1;
+    return got;
 }
 
 static int