From 8b60ff7f103cab6c31db96386ee62e80aa5e0e45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1n=20Tomko?= Date: Mon, 3 Dec 2012 13:35:05 +0100 Subject: [PATCH] conf: prevent crash with no uuid in cephx auth secret Fix the null pointer access when UUID is not specified. Introduce a bool 'uuidUsable' to virStoragePoolAuthCephx that indicates if uuid was specified or not and use it instead of the pointless comparison of the static UUID array to NULL. Add an error message if both uuid and usage are specified. Fixes: Error: FORWARD_NULL (CWE-476): libvirt-0.10.2/src/conf/storage_conf.c:461: var_deref_model: Passing null pointer "uuid" to function "virUUIDParse(char const *, unsigned char *)", which dereferences it. (The dereference is assumed on the basis of the 'nonnull' parameter attribute.) Error: NO_EFFECT (CWE-398): libvirt-0.10.2/src/conf/storage_conf.c:979: array_null: Comparing an array to null is not useful: "src->auth.cephx.secret.uuid != NULL". (cherry picked from commit bc680e1381be7eeb5ae7d898ebab598df819b672) --- src/conf/storage_conf.c | 20 +++++++++++++++----- src/conf/storage_conf.h | 1 + src/storage/storage_backend_rbd.c | 6 ++---- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 4daf059648..f8587dd77f 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -449,10 +449,20 @@ virStoragePoolDefParseAuthCephx(xmlXPathContextPtr ctxt, return -1; } - if (virUUIDParse(uuid, auth->secret.uuid) < 0) { - virReportError(VIR_ERR_XML_ERROR, - "%s", _("invalid auth secret uuid")); - return -1; + if (uuid != NULL) { + if (auth->secret.usage != NULL) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("either auth secret uuid or usage expected")); + return -1; + } + if (virUUIDParse(uuid, auth->secret.uuid) < 0) { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("invalid auth secret uuid")); + return -1; + } + auth->secret.uuidUsable = true; + } else { + auth->secret.uuidUsable = false; } return 0; @@ -967,7 +977,7 @@ virStoragePoolSourceFormat(virBufferPtr buf, src->auth.cephx.username); virBufferAsprintf(buf," %s", "auth.cephx.secret.uuid != NULL) { + if (src->auth.cephx.secret.uuidUsable) { virUUIDFormat(src->auth.cephx.secret.uuid, uuid); virBufferAsprintf(buf," uuid='%s'", uuid); } diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index d509b135c3..743b768d5f 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -169,6 +169,7 @@ struct _virStoragePoolAuthCephx { struct { unsigned char uuid[VIR_UUID_BUFLEN]; char *usage; + bool uuidUsable; } secret; }; diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 767892f0eb..2f18e91b57 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -70,13 +70,11 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr *ptr, goto cleanup; } - if (pool->def->source.auth.cephx.secret.uuid != NULL) { + if (pool->def->source.auth.cephx.secret.uuidUsable) { virUUIDFormat(pool->def->source.auth.cephx.secret.uuid, secretUuid); VIR_DEBUG("Looking up secret by UUID: %s", secretUuid); secret = virSecretLookupByUUIDString(conn, secretUuid); - } - - if (pool->def->source.auth.cephx.secret.usage != NULL) { + } else if (pool->def->source.auth.cephx.secret.usage != NULL) { VIR_DEBUG("Looking up secret by usage: %s", pool->def->source.auth.cephx.secret.usage); secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_CEPH,