From 45d6729f98e9842b139b809078d43f1f7a8c779b Mon Sep 17 00:00:00 2001 From: Jiri Denemark Date: Mon, 30 Jul 2012 12:14:54 +0200 Subject: [PATCH] daemon: Fix crash in virTypedParameterArrayClear CVE-2012-3445, https://bugzilla.redhat.com/show_bug.cgi?id=844745 Daemon uses the following pattern when dispatching APIs with typed parameters: VIR_ALLOC_N(params, nparams); virDomain*(dom, params, &nparams, flags); virTypedParameterArrayClear(params, nparams); In case nparams was originally set to 0, virDomain* API would fill it with the number of typed parameters it can provide and we would use this number (rather than zero) to clear params. Because VIR_ALLOC* returns non-NULL pointer even if size is 0, the code would end up walking through random memory. If we were lucky enough and the memory contained 7 (VIR_TYPED_PARAM_STRING) at the right place, we would try to free a random pointer and crash. Let's make sure params stays NULL when nparams is 0. (cherry picked from commit 6039a2cb49c8af4c68460d2faf365a7e1c686c7b) --- daemon/remote.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 74811a4ea3..0cb49a001e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -964,7 +964,7 @@ remoteDispatchDomainGetSchedulerParameters(virNetServerPtr server ATTRIBUTE_UNUS virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); goto cleanup; } - if (VIR_ALLOC_N(params, nparams) < 0) + if (nparams && VIR_ALLOC_N(params, nparams) < 0) goto no_memory; if (!(dom = get_nonnull_domain(priv->conn, args->dom))) @@ -1019,7 +1019,7 @@ remoteDispatchDomainGetSchedulerParametersFlags(virNetServerPtr server ATTRIBUTE virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); goto cleanup; } - if (VIR_ALLOC_N(params, nparams) < 0) + if (nparams && VIR_ALLOC_N(params, nparams) < 0) goto no_memory; if (!(dom = get_nonnull_domain(priv->conn, args->dom))) @@ -1200,7 +1200,7 @@ remoteDispatchDomainBlockStatsFlags(virNetServerPtr server ATTRIBUTE_UNUSED, virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); goto cleanup; } - if (VIR_ALLOC_N(params, nparams) < 0) { + if (nparams && VIR_ALLOC_N(params, nparams) < 0) { virReportOOMError(); goto cleanup; } @@ -1674,7 +1674,7 @@ remoteDispatchDomainGetMemoryParameters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); goto cleanup; } - if (VIR_ALLOC_N(params, nparams) < 0) { + if (nparams && VIR_ALLOC_N(params, nparams) < 0) { virReportOOMError(); goto cleanup; } @@ -1739,7 +1739,7 @@ remoteDispatchDomainGetNumaParameters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); goto cleanup; } - if (VIR_ALLOC_N(params, nparams) < 0) { + if (nparams && VIR_ALLOC_N(params, nparams) < 0) { virReportOOMError(); goto cleanup; } @@ -1804,7 +1804,7 @@ remoteDispatchDomainGetBlkioParameters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); goto cleanup; } - if (VIR_ALLOC_N(params, nparams) < 0) { + if (nparams && VIR_ALLOC_N(params, nparams) < 0) { virReportOOMError(); goto cleanup; } @@ -2064,7 +2064,7 @@ remoteDispatchDomainGetBlockIoTune(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; } - if (VIR_ALLOC_N(params, nparams) < 0) { + if (nparams && VIR_ALLOC_N(params, nparams) < 0) { virReportOOMError(); goto cleanup; } @@ -3563,7 +3563,7 @@ remoteDispatchDomainGetInterfaceParameters(virNetServerPtr server ATTRIBUTE_UNUS virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); goto cleanup; } - if (VIR_ALLOC_N(params, nparams) < 0) { + if (nparams && VIR_ALLOC_N(params, nparams) < 0) { virReportOOMError(); goto cleanup; }