[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



The trouble here is that "goto failure" would run this code:

  failure:
    VIR_FREE(*datastoreName);
    VIR_FREE(*directoryName);
    VIR_FREE(*fileName);
    result = -1;
    goto cleanup;

And thus if any of those 3 input variables was NULL,
that deref would probably provoke a segfault.

By the way, that interface should be changed,
because it encourages risky behavior:

  int
  esxUtil_ParseDatastoreRelatedPath(virConnectPtr conn,
                                    const char *datastoreRelatedPath,
                                    char **datastoreName,
                                    char **directoryName, char **fileName)

Note how it takes a buffer, datastoreRelatedPath,
but with no length parameter.  Then this function
writes into the buffer with this code:

    if (sscanf(datastoreRelatedPath, "[%a[^]%]] %a[^\n]", datastoreName,
               &directoryAndFileName) != 2) {

which cannot even check for buffer overrun because it doesn't
know the length.

You might want to add a new parameter, buf_len:

  int
  esxUtil_ParseDatastoreRelatedPath(virConnectPtr conn,
                                    const char *datastoreRelatedPath,
                                    size_t buf_len,
                                    char **datastoreName,
                                    char **directoryName, char **fileName)

>From aba304e07835c123bd63f1d5d6777a898916349f Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering 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, rather than trying to clean up and dereference NULL
pointers, if any input is invalid.
---
 src/esx/esx_util.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 3e53921..8703ac2 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;
     }

     /*
@@ -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);
-        goto failure;
+        return -1;
     }

     /* Split <path> into <directory>/<file>, where <directory> is optional */
--
1.6.6.rc2.275.g51e2d


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]