From 6324949fb11ce0148f34410f0ebf8f1a69ad82da Mon Sep 17 00:00:00 2001
From: Eduardo Otubo <otubo@linux.vnet.ibm.com>
Date: Thu, 15 Jul 2010 19:37:48 -0300
Subject: [PATCH] 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.
---
 src/phyp/phyp_driver.c | 47 +++++++++++++++---------------------------
 1 file changed, 17 insertions(+), 30 deletions(-)

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++;
-        }
-    }
 
-    VIR_FREE(cmd);
-    VIR_FREE(ret);
-    return got;
+    /* 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 */
+    }
 
   err:
     VIR_FREE(cmd);
     VIR_FREE(ret);
-    return -1;
+    return got;
 }
 
 static int