On 11/7/25 12:26, Martin Kletzander via Devel wrote:
> From: Martin Kletzander <[email protected]>
>
> Utilise the new virDomainDefIDsParseString() for that.
>
> Fixes: CVE-2025-12748
> Reported-by: Святослав Терешин <[email protected]>
> Signed-off-by: Martin Kletzander <[email protected]>
> ---
> 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
