From nobody Thu May 15 11:44:37 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1507274047780134.00417461252653; Fri, 6 Oct 2017 00:14:07 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4691F3DBE0; Fri, 6 Oct 2017 07:14:06 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B008A669EE; Fri, 6 Oct 2017 07:14:05 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id CD1F31806104; Fri, 6 Oct 2017 07:14:04 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v966lpfi020260 for ; Fri, 6 Oct 2017 02:47:51 -0400 Received: by smtp.corp.redhat.com (Postfix) id F23BD6135F; Fri, 6 Oct 2017 06:47:50 +0000 (UTC) Received: from dhcp-1-107.brq.redhat.com (ovpn-204-170.brq.redhat.com [10.40.204.170]) by smtp.corp.redhat.com (Postfix) with ESMTP id E96EF61359; Fri, 6 Oct 2017 06:47:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 4691F3DBE0 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com From: Ladi Prosek To: libvir-list@redhat.com Date: Fri, 6 Oct 2017 08:47:34 +0200 Message-Id: <20171006064735.29638-3-lprosek@redhat.com> In-Reply-To: <20171006064735.29638-1-lprosek@redhat.com> References: <20171006064735.29638-1-lprosek@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 2/3] hyperv: Escape WQL queries X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 06 Oct 2017 07:14:06 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" The code was vulnerable to SQL injection. Likely not a security issue due to WMI SQL and other constraints but still lame. For example: virsh # dominfo \" error: failed to get domain '"' error: internal error: SOAP fault during enumeration: code 's:Sender', su= bcode 'n:CannotProcessFilter', reason 'The data source could not process the fi= lter. The filter might be missing or it might be invalid. Change the filter and= try the request again. ', detail 'The WS-Management service cannot process t= he request. The WQL query is invalid. ' This commit fixes the Hyper-V driver by escaping all WMI SQL string paramet= ers. The same command with the fix: virsh # dominfo \" error: failed to get domain '"' error: Domain not found: No domain with name " Signed-off-by: Ladi Prosek Reviewed-by: John Ferlan --- src/hyperv/hyperv_driver.c | 96 +++++++++++++++++++++++-------------------= ---- src/hyperv/hyperv_wmi.c | 2 +- src/util/virbuffer.c | 18 +++++++++ src/util/virbuffer.h | 3 ++ 4 files changed, 70 insertions(+), 49 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 52274239c..998780b80 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -302,12 +302,12 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr = info) } =20 /* Get Win32_Processor list */ - virBufferAsprintf(&query, - "associators of " - "{Win32_ComputerSystem.Name=3D\"%s\"} " - "where AssocClass =3D Win32_ComputerSystemProcessor " - "ResultClass =3D Win32_Processor", - computerSystem->data.common->Name); + virBufferEscapeSQL(&query, + "associators of " + "{Win32_ComputerSystem.Name=3D\"%s\"} " + "where AssocClass =3D Win32_ComputerSystemProcessor= " + "ResultClass =3D Win32_Processor", + computerSystem->data.common->Name); =20 if (hypervGetWin32ProcessorList(priv, &query, &processorList) < 0) goto cleanup; @@ -494,7 +494,7 @@ hypervDomainLookupByUUID(virConnectPtr conn, const unsi= gned char *uuid) virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); virBufferAddLit(&query, "where "); virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); - virBufferAsprintf(&query, "and Name =3D \"%s\"", uuid_string); + virBufferEscapeSQL(&query, "and Name =3D \"%s\"", uuid_string); =20 if (hypervGetMsvmComputerSystemList(priv, &query, &computerSystem) < 0) goto cleanup; @@ -526,7 +526,7 @@ hypervDomainLookupByName(virConnectPtr conn, const char= *name) virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); virBufferAddLit(&query, "where "); virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); - virBufferAsprintf(&query, "and ElementName =3D \"%s\"", name); + virBufferEscapeSQL(&query, "and ElementName =3D \"%s\"", name); =20 if (hypervGetMsvmComputerSystemList(priv, &query, &computerSystem) < 0) goto cleanup; @@ -674,13 +674,13 @@ hypervDomainGetInfo(virDomainPtr domain, virDomainInf= oPtr info) goto cleanup; =20 /* Get Msvm_VirtualSystemSettingData */ - virBufferAsprintf(&query, - "associators of " - "{Msvm_ComputerSystem.CreationClassName=3D\"Msvm_Com= puterSystem\"," - "Name=3D\"%s\"} " - "where AssocClass =3D Msvm_SettingsDefineState " - "ResultClass =3D Msvm_VirtualSystemSettingData", - uuid_string); + virBufferEscapeSQL(&query, + "associators of " + "{Msvm_ComputerSystem.CreationClassName=3D\"Msvm_Co= mputerSystem\"," + "Name=3D\"%s\"} " + "where AssocClass =3D Msvm_SettingsDefineState " + "ResultClass =3D Msvm_VirtualSystemSettingData", + uuid_string); =20 if (hypervGetMsvmVirtualSystemSettingDataList(priv, &query, &virtualSystemSettingDat= a) < 0) { @@ -696,12 +696,12 @@ hypervDomainGetInfo(virDomainPtr domain, virDomainInf= oPtr info) } =20 /* Get Msvm_ProcessorSettingData */ - virBufferAsprintf(&query, - "associators of " - "{Msvm_VirtualSystemSettingData.InstanceID=3D\"%s\"}= " - "where AssocClass =3D Msvm_VirtualSystemSettingDataC= omponent " - "ResultClass =3D Msvm_ProcessorSettingData", - virtualSystemSettingData->data.common->InstanceID); + virBufferEscapeSQL(&query, + "associators of " + "{Msvm_VirtualSystemSettingData.InstanceID=3D\"%s\"= } " + "where AssocClass =3D Msvm_VirtualSystemSettingData= Component " + "ResultClass =3D Msvm_ProcessorSettingData", + virtualSystemSettingData->data.common->InstanceID); =20 if (hypervGetMsvmProcessorSettingDataList(priv, &query, &processorSettingData) < 0) { @@ -717,12 +717,12 @@ hypervDomainGetInfo(virDomainPtr domain, virDomainInf= oPtr info) } =20 /* Get Msvm_MemorySettingData */ - virBufferAsprintf(&query, - "associators of " - "{Msvm_VirtualSystemSettingData.InstanceID=3D\"%s\"}= " - "where AssocClass =3D Msvm_VirtualSystemSettingDataC= omponent " - "ResultClass =3D Msvm_MemorySettingData", - virtualSystemSettingData->data.common->InstanceID); + virBufferEscapeSQL(&query, + "associators of " + "{Msvm_VirtualSystemSettingData.InstanceID=3D\"%s\"= } " + "where AssocClass =3D Msvm_VirtualSystemSettingData= Component " + "ResultClass =3D Msvm_MemorySettingData", + virtualSystemSettingData->data.common->InstanceID); =20 if (hypervGetMsvmMemorySettingDataList(priv, &query, &memorySettingData) < 0) { @@ -811,13 +811,13 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned = int flags) goto cleanup; =20 /* Get Msvm_VirtualSystemSettingData */ - virBufferAsprintf(&query, - "associators of " - "{Msvm_ComputerSystem.CreationClassName=3D\"Msvm_Com= puterSystem\"," - "Name=3D\"%s\"} " - "where AssocClass =3D Msvm_SettingsDefineState " - "ResultClass =3D Msvm_VirtualSystemSettingData", - uuid_string); + virBufferEscapeSQL(&query, + "associators of " + "{Msvm_ComputerSystem.CreationClassName=3D\"Msvm_Co= mputerSystem\"," + "Name=3D\"%s\"} " + "where AssocClass =3D Msvm_SettingsDefineState " + "ResultClass =3D Msvm_VirtualSystemSettingData", + uuid_string); =20 if (hypervGetMsvmVirtualSystemSettingDataList(priv, &query, &virtualSystemSettingDat= a) < 0) { @@ -833,12 +833,12 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned = int flags) } =20 /* Get Msvm_ProcessorSettingData */ - virBufferAsprintf(&query, - "associators of " - "{Msvm_VirtualSystemSettingData.InstanceID=3D\"%s\"}= " - "where AssocClass =3D Msvm_VirtualSystemSettingDataC= omponent " - "ResultClass =3D Msvm_ProcessorSettingData", - virtualSystemSettingData->data.common->InstanceID); + virBufferEscapeSQL(&query, + "associators of " + "{Msvm_VirtualSystemSettingData.InstanceID=3D\"%s\"= } " + "where AssocClass =3D Msvm_VirtualSystemSettingData= Component " + "ResultClass =3D Msvm_ProcessorSettingData", + virtualSystemSettingData->data.common->InstanceID); =20 if (hypervGetMsvmProcessorSettingDataList(priv, &query, &processorSettingData) < 0) { @@ -854,12 +854,12 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned = int flags) } =20 /* Get Msvm_MemorySettingData */ - virBufferAsprintf(&query, - "associators of " - "{Msvm_VirtualSystemSettingData.InstanceID=3D\"%s\"}= " - "where AssocClass =3D Msvm_VirtualSystemSettingDataC= omponent " - "ResultClass =3D Msvm_MemorySettingData", - virtualSystemSettingData->data.common->InstanceID); + virBufferEscapeSQL(&query, + "associators of " + "{Msvm_VirtualSystemSettingData.InstanceID=3D\"%s\"= } " + "where AssocClass =3D Msvm_VirtualSystemSettingData= Component " + "ResultClass =3D Msvm_MemorySettingData", + virtualSystemSettingData->data.common->InstanceID); =20 if (hypervGetMsvmMemorySettingDataList(priv, &query, &memorySettingData) < 0) { @@ -1398,7 +1398,7 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int= codeset, if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) goto cleanup; =20 - virBufferAsprintf(&query, + virBufferEscapeSQL(&query, "associators of " "{Msvm_ComputerSystem.CreationClassName=3D\"Msvm_ComputerSyste= m\"," "Name=3D\"%s\"} " @@ -1535,7 +1535,7 @@ hypervDomainSetMemoryFlags(virDomainPtr domain, unsig= ned long memory, } =20 virBufferAddLit(&eprQuery, MSVM_COMPUTERSYSTEM_WQL_SELECT); - virBufferAsprintf(&eprQuery, "where Name =3D \"%s\"", uuid_string); + virBufferEscapeSQL(&eprQuery, "where Name =3D \"%s\"", uuid_string= ); =20 if (hypervAddEprParam(params, "ComputerSystem", priv, &eprQuery, Msvm_ComputerSystem_WmiInfo) < 0) diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 0b9431bfa..5e2b3d7ed 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -895,7 +895,7 @@ hypervInvokeMethod(hypervPrivate *priv, hypervInvokePar= amsListPtr params, */ while (!completed && timeout >=3D 0) { virBufferAddLit(&query, MSVM_CONCRETEJOB_WQL_SELECT); - virBufferAsprintf(&query, "where InstanceID =3D \"%s\"", insta= nceID); + virBufferEscapeSQL(&query, "where InstanceID =3D \"%s\"", inst= anceID); =20 if (hypervGetMsvmConcreteJobList(priv, &query, &job) < 0 || job =3D=3D NULL) diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c index 28a291bb0..15f6e21fb 100644 --- a/src/util/virbuffer.c +++ b/src/util/virbuffer.c @@ -575,6 +575,24 @@ virBufferEscapeRegex(virBufferPtr buf, } =20 /** + * virBufferEscapeSQL: + * @buf: the buffer to append to + * @format: a printf like format string but with only one %s parameter + * @str: the string argument which needs to be escaped + * + * Do a formatted print with a single string to a buffer. The @str is + * escaped to prevent SQL injection (format is expected to contain \"%s\"). + * Auto indentation may be applied. + */ +void +virBufferEscapeSQL(virBufferPtr buf, + const char *format, + const char *str) +{ + virBufferEscape(buf, '\\', "'\"\\", format, str); +} + +/** * virBufferEscape: * @buf: the buffer to append to * @escape: the escape character to inject diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h index 3211c07b0..4f5ed162f 100644 --- a/src/util/virbuffer.h +++ b/src/util/virbuffer.h @@ -93,6 +93,9 @@ void virBufferEscapeSexpr(virBufferPtr buf, const char *f= ormat, void virBufferEscapeRegex(virBufferPtr buf, const char *format, const char *str); +void virBufferEscapeSQL(virBufferPtr buf, + const char *format, + const char *str); void virBufferEscapeShell(virBufferPtr buf, const char *str); void virBufferURIEncodeString(virBufferPtr buf, const char *str); =20 --=20 2.13.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list