From: Ján Tomko <jtomko@redhat.com>
Date: Mon, 3 Dec 2012 12:35:05 +0000 (+0100)
Subject: conf: prevent crash with no uuid in cephx auth secret
X-Git-Tag: v0.10.2.2~7
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8b60ff7f103cab6c31db96386ee62e80aa5e0e45;p=thirdparty%2Flibvirt.git

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

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", "<secret");
-        if (src->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,