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(a)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