From: Martin Kletzander <mkletzan@redhat.com>

Utilise the new virDomainDefIDsParseString() for that.

Fixes: CVE-2025-12748
Reported-by: Святослав Терешин <s.tereshin@fobos-nt.ru>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/bhyve/bhyve_driver.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 00a484ae219c..72f1d7ace8e6 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -486,6 +486,15 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag
     if (!caps)
         return NULL;
 
+    /* Avoid parsing the whole domain definition for ACL checks */
+    if (!(def = virDomainDefIDsParseString(xml, provconn->xmlopt, parse_flags)))
+        return NULL;
+
+    if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
+        return NULL;
+
+    g_clear_pointer(&def, g_object_unref);
+
     if ((def = virDomainDefParseString(xml, privconn->xmlopt,
                                        NULL, parse_flags)) == NULL)
         goto cleanup;
@@ -493,9 +502,6 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag
     if (virXMLCheckIllegalChars("name", def->name, "\n") < 0)
         goto cleanup;
 
-    if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
-        goto cleanup;
-
     if (bhyveDomainAssignAddresses(def, NULL) < 0)
         goto cleanup;
 
@@ -889,11 +895,17 @@ bhyveDomainCreateXML(virConnectPtr conn,
     if (flags & VIR_DOMAIN_START_AUTODESTROY)
         start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY;
 
-    if ((def = virDomainDefParseString(xml, privconn->xmlopt,
-                                       NULL, parse_flags)) == NULL)
-        goto cleanup;
+    /* Avoid parsing the whole domain definition for ACL checks */
+    if (!(def = virDomainDefIDsParseString(xml, provconn->xmlopt, parse_flags)))
+        return NULL;
 
     if (virDomainCreateXMLEnsureACL(conn, def) < 0)
+        return NULL;
+
+    g_clear_pointer(&def, g_object_unref);
+
+    if ((def = virDomainDefParseString(xml, privconn->xmlopt,
+                                       NULL, parse_flags)) == NULL)
         goto cleanup;
 
     if (bhyveDomainAssignAddresses(def, NULL) < 0)
-- 
2.51.2

On 11/7/25 12:26, Martin Kletzander via Devel wrote:
> From: Martin Kletzander <mkletzan@redhat.com>
> 
> Utilise the new virDomainDefIDsParseString() for that.
> 
> Fixes: CVE-2025-12748
> Reported-by: Святослав Терешин <s.tereshin@fobos-nt.ru>
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/bhyve/bhyve_driver.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index 00a484ae219c..72f1d7ace8e6 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -486,6 +486,15 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag
>      if (!caps)
>          return NULL;
>  
> +    /* Avoid parsing the whole domain definition for ACL checks */
> +    if (!(def = virDomainDefIDsParseString(xml, provconn->xmlopt, parse_flags)))
> +        return NULL;
> +
> +    if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
> +        return NULL;
> +
> +    g_clear_pointer(&def, g_object_unref);

Haven't checked other patches yet, but virDomainDef is NOT an virObject.
This should have been:

  g_clear_pointer(&def, virDomainDefFree);

Here and in the rest of patches.

> +
>      if ((def = virDomainDefParseString(xml, privconn->xmlopt,
>                                         NULL, parse_flags)) == NULL)
>          goto cleanup;
> @@ -493,9 +502,6 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag
>      if (virXMLCheckIllegalChars("name", def->name, "\n") < 0)
>          goto cleanup;

So previously, illegal characters were checked for before ACL check and
now they are checked for afterwards. But I think that's okay - the order
shouldn't matter.

>  
> -    if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
> -        goto cleanup;
> -
>      if (bhyveDomainAssignAddresses(def, NULL) < 0)
>          goto cleanup;
>  
> @@ -889,11 +895,17 @@ bhyveDomainCreateXML(virConnectPtr conn,
>      if (flags & VIR_DOMAIN_START_AUTODESTROY)
>          start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY;
>  
> -    if ((def = virDomainDefParseString(xml, privconn->xmlopt,
> -                                       NULL, parse_flags)) == NULL)
> -        goto cleanup;
> +    /* Avoid parsing the whole domain definition for ACL checks */
> +    if (!(def = virDomainDefIDsParseString(xml, provconn->xmlopt, parse_flags)))
> +        return NULL;
>  
>      if (virDomainCreateXMLEnsureACL(conn, def) < 0)
> +        return NULL;
> +
> +    g_clear_pointer(&def, g_object_unref);
> +
> +    if ((def = virDomainDefParseString(xml, privconn->xmlopt,
> +                                       NULL, parse_flags)) == NULL)
>          goto cleanup;
>  
>      if (bhyveDomainAssignAddresses(def, NULL) < 0)


Michal

On Tue, Nov 11, 2025 at 09:54:22AM +0100, Michal Prívozník wrote:
>On 11/7/25 12:26, Martin Kletzander via Devel wrote:
>> From: Martin Kletzander <mkletzan@redhat.com>
>>
>> Utilise the new virDomainDefIDsParseString() for that.
>>
>> Fixes: CVE-2025-12748
>> Reported-by: Святослав Терешин <s.tereshin@fobos-nt.ru>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  src/bhyve/bhyve_driver.c | 24 ++++++++++++++++++------
>>  1 file changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
>> index 00a484ae219c..72f1d7ace8e6 100644
>> --- a/src/bhyve/bhyve_driver.c
>> +++ b/src/bhyve/bhyve_driver.c
>> @@ -486,6 +486,15 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag
>>      if (!caps)
>>          return NULL;
>>
>> +    /* Avoid parsing the whole domain definition for ACL checks */
>> +    if (!(def = virDomainDefIDsParseString(xml, provconn->xmlopt, parse_flags)))
>> +        return NULL;
>> +
>> +    if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
>> +        return NULL;
>> +
>> +    g_clear_pointer(&def, g_object_unref);
>
>Haven't checked other patches yet, but virDomainDef is NOT an virObject.
>This should have been:
>
>  g_clear_pointer(&def, virDomainDefFree);
>
>Here and in the rest of patches.
>

Well, that's a stupid mistake, thanks for catching that.  Fixed locally.