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".

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 3fdc5b639c483e49581035c82574c83622c77547..a6c6ce7104bdee85b943d4207dc747ec1c5488fa 100644 (file)
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -458,10 +458,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;
@@ -979,7 +989,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 d509b135c36742bf15dda828c727d6cc9b0d08d9..743b768d5f085c6bc7386df946cfba135a1a2565 100644 (file)
--- 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 0c9bdccea4cc88625cf5dc235e061a30a1f60d29..bc61cf7dfa267ed6272bf3495d9ed83a3c19e0c8 100644 (file)
--- 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,