[libvirt] [PATCH] esx_util.c: avoid NULL deref for invalid inputs

[Jim Meyering]

Jim Meyering wrote:
>>>    if (sscanf(datastoreRelatedPath, "[%a[^]%]] %a[^\n]", datastoreName,
>>>               &directoryAndFileName) != 2) {
...
I suppose you know that %a[...] is non-standard (it's a glibc-specific
addition) and that this esx code need only compile on systems with glibc.

I confirm that with my change that would have leaked.
Though note that it would do so only when parsing exactly one token.

>> This fix is not correct, it may leak memory allocated by sscanf. goto
>> failure is correct and safe here, because the first if block in this
>> function has check the char ** pointers to be not-NULL, so the
>> VIR_FREEs after the failure label is safe.
>
> Good catch.
> I'll adjust and resend.

I've folded in this correction:

diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 8703ac2..35a48e0 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -299,7 +299,7 @@ esxUtil_ParseDatastoreRelatedPath(virConnectPtr conn,
         ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR,
                   "Datastore related path '%s' doesn't have expected format "
                   "'[<datastore>] <path>'", datastoreRelatedPath);
-        return -1;
+        goto failure;
     }

     /* Split <path> into <directory>/<file>, where <directory> is optional */

Here's the result:

>From f34c4395b9b16965705f9158ac90e59c37b04507 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Tue, 15 Dec 2009 19:08:49 +0100
Subject: [PATCH] esx_util.c: avoid NULL deref for invalid inputs

* src/esx/esx_util.c (esxUtil_ParseDatastoreRelatedPath): Return
right away for invalid inputs, rather than using them (which would
dereference NULL pointers) in clean-up code.
---
 src/esx/esx_util.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 3e53921..35a48e0 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -277,7 +277,7 @@ esxUtil_ParseDatastoreRelatedPath(virConnectPtr conn,
         directoryName == NULL || *directoryName != NULL ||
         fileName == NULL || *fileName != NULL) {
         ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument");
-        goto failure;
+        return -1;
     }

     /*
--
1.6.6.rc2.275.g51e2d