From nobody Sat Apr 27 02:22:00 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; spf=none (zoho.com: 198.145.21.10 is neither permitted nor denied by domain of lists.01.org) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail(p=none dis=none) header.from=intel.com Return-Path: Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx.zohomail.com with SMTPS id 1533014162815529.4883622219562; Mon, 30 Jul 2018 22:16:02 -0700 (PDT) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id A0C3D210C6444; Mon, 30 Jul 2018 22:16:01 -0700 (PDT) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 2C02A2098EA7D for ; Mon, 30 Jul 2018 22:16:00 -0700 (PDT) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 Jul 2018 22:15:59 -0700 Received: from ydong10-win10.ccr.corp.intel.com ([10.239.9.125]) by orsmga001.jf.intel.com with ESMTP; 30 Jul 2018 22:15:58 -0700 X-Original-To: edk2-devel@lists.01.org Received-SPF: none (zoho.com: 198.145.21.10 is neither permitted nor denied by domain of lists.01.org) client-ip=198.145.21.10; envelope-from=edk2-devel-bounces@lists.01.org; helo=ml01.01.org; Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.126; helo=mga18.intel.com; envelope-from=eric.dong@intel.com; receiver=edk2-devel@lists.01.org X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,426,1526367600"; d="scan'208";a="77387615" From: Eric Dong To: edk2-devel@lists.01.org Date: Tue, 31 Jul 2018 13:15:56 +0800 Message-Id: <20180731051556.15980-1-eric.dong@intel.com> X-Mailer: git-send-email 2.15.0.windows.1 Subject: [edk2] [Patch] [UDK2017] SecurityPkg OpalPasswordSupportLib: Add check to avoid potential buffer overflow. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Wu Hao , Yao Jiewen MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" X-ZohoMail: RSF_4 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Current code not check the CommunicationBuffer size before use it. Attacker= can read beyond the end of the (untrusted) commbuffer into controlled memory. A= ttacker can get access outside of valid SMM memory regions. This patch add check be= fore use it. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Eric Dong Cc: Yao Jiewen Cc: Wu Hao Reviewed-by: jiewen.yao@intel.com --- .../Include/Library/OpalPasswordSupportLib.h | 3 +- .../OpalPasswordSupportLib.c | 55 +++++++++++++++---= ---- .../OpalPasswordSupportNotify.h | 2 +- .../Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.h | 6 +-- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/SecurityPkg/Include/Library/OpalPasswordSupportLib.h b/Securit= yPkg/Include/Library/OpalPasswordSupportLib.h index e616c763f0..ad45e9e3b7 100644 --- a/SecurityPkg/Include/Library/OpalPasswordSupportLib.h +++ b/SecurityPkg/Include/Library/OpalPasswordSupportLib.h @@ -19,6 +19,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER= EXPRESS OR IMPLIED. #include #include =20 +#define OPAL_PASSWORD_MAX_LENGTH 32 =20 #pragma pack(1) =20 @@ -76,7 +77,7 @@ typedef struct { typedef struct { LIST_ENTRY Link; =20 - UINT8 Password[32]; + UINT8 Password[OPAL_PASSWORD_MAX_LENGTH]; UINT8 PasswordLength; =20 EFI_DEVICE_PATH_PROTOCOL OpalDevicePath; diff --git a/SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupport= Lib.c b/SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.c index 837582359e..e377e9ca79 100644 --- a/SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.c +++ b/SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.c @@ -14,8 +14,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER= EXPRESS OR IMPLIED. =20 #include "OpalPasswordSupportNotify.h" =20 -#define OPAL_PASSWORD_MAX_LENGTH 32 - LIST_ENTRY mDeviceList =3D INITIALIZE_LIST_HEAD_VARIABLE (mDevic= eList); BOOLEAN gInSmm =3D FALSE; EFI_GUID gOpalPasswordNotifyProtocolGuid =3D OPAL_PASSWORD_NOT= IFY_PROTOCOL_GUID; @@ -663,34 +661,53 @@ SmmOpalPasswordHandler ( EFI_STATUS Status; OPAL_SMM_COMMUNICATE_HEADER *SmmFunctionHeader; UINTN TempCommBufferSize; - UINT8 *NewPassword; - UINT8 PasswordLength; - EFI_DEVICE_PATH_PROTOCOL *DevicePath; + UINTN RemainedDevicePathSize; + OPAL_COMM_DEVICE_LIST *DeviceBuffer; =20 if (CommBuffer =3D=3D NULL || CommBufferSize =3D=3D NULL) { return EFI_SUCCESS; } =20 + Status =3D EFI_SUCCESS; + DeviceBuffer =3D NULL; + TempCommBufferSize =3D *CommBufferSize; if (TempCommBufferSize < OFFSET_OF (OPAL_SMM_COMMUNICATE_HEADER, Data)) { return EFI_SUCCESS; } - - Status =3D EFI_SUCCESS; - SmmFunctionHeader =3D (OPAL_SMM_COMMUNICATE_HEADER *)CommBuffer; - - DevicePath =3D &((OPAL_COMM_DEVICE_LIST*)(SmmFunctionHeader->Data))->Opa= lDevicePath; - PasswordLength =3D ((OPAL_COMM_DEVICE_LIST*)(SmmFunctionHeader->Data))->= PasswordLength; - NewPassword =3D ((OPAL_COMM_DEVICE_LIST*)(SmmFunctionHeader->Data))->Pas= sword; + SmmFunctionHeader =3D (OPAL_SMM_COMMUNICATE_HEADER *)CommBuffer; =20 switch (SmmFunctionHeader->Function) { case SMM_FUNCTION_SET_OPAL_PASSWORD: - if (OpalPasswordIsFullZero (NewPassword) || PasswordLength =3D=3D = 0) { - Status =3D EFI_INVALID_PARAMETER; - goto EXIT; - } + if (TempCommBufferSize <=3D OFFSET_OF (OPAL_SMM_COMMUNICATE_HEADER, = Data) + OFFSET_OF (OPAL_COMM_DEVICE_LIST, OpalDevicePath)) { + return EFI_SUCCESS; + } + + DeviceBuffer =3D AllocateCopyPool (TempCommBufferSize - OFFSET_OF (O= PAL_SMM_COMMUNICATE_HEADER, Data), SmmFunctionHeader->Data); + if (DeviceBuffer =3D=3D NULL) { + Status =3D EFI_OUT_OF_RESOURCES; + goto EXIT; + } =20 - Status =3D OpalSavePasswordToSmm (DevicePath, NewPassword, Passwor= dLength); + // + // Validate the DevicePath. + // + RemainedDevicePathSize =3D TempCommBufferSize - OFFSET_OF (OPAL_SMM_= COMMUNICATE_HEADER, Data) + - OFFSET_OF (OPAL_COMM_DEVICE_LIST, Op= alDevicePath); + if (!IsDevicePathValid(&DeviceBuffer->OpalDevicePath, RemainedDevice= PathSize) || + (RemainedDevicePathSize !=3D GetDevicePathSize (&DeviceBuffer->O= palDevicePath))) { + Status =3D EFI_SUCCESS; + goto EXIT; + } + + if (OpalPasswordIsFullZero (DeviceBuffer->Password) || + DeviceBuffer->PasswordLength =3D=3D 0 || + DeviceBuffer->PasswordLength > OPAL_PASSWORD_MAX_LENGTH) { + Status =3D EFI_INVALID_PARAMETER; + goto EXIT; + } + + Status =3D OpalSavePasswordToSmm (&DeviceBuffer->OpalDevicePath, Dev= iceBuffer->Password, DeviceBuffer->PasswordLength); break; =20 default: @@ -701,6 +718,10 @@ SmmOpalPasswordHandler ( EXIT: SmmFunctionHeader->ReturnStatus =3D Status; =20 + if (DeviceBuffer !=3D NULL) { + FreePool (DeviceBuffer); + } + // // Return EFI_SUCCESS cause only one handler can be trigged. // so return EFI_WARN_INTERRUPT_SOURCE_PENDING to make all handler can b= e trigged. diff --git a/SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupport= Notify.h b/SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportNo= tify.h index f0ad3a1136..d543faed5d 100644 --- a/SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportNotify.h +++ b/SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportNotify.h @@ -41,7 +41,7 @@ typedef struct { } OPAL_SMM_COMMUNICATE_HEADER; =20 typedef struct { - UINT8 Password[32]; + UINT8 Password[OPAL_PASSWORD_MAX_LENGTH]; UINT8 PasswordLength; =20 EFI_DEVICE_PATH_PROTOCOL OpalDevicePath; diff --git a/SecurityPkg/Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.h b/Secur= ityPkg/Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.h index ce88786fab..bc559f0bd1 100644 --- a/SecurityPkg/Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.h +++ b/SecurityPkg/Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.h @@ -64,10 +64,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHE= R EXPRESS OR IMPLIED. // The payload Length of HDD related ATA commands // #define HDD_PAYLOAD 512 -// -// According to ATA spec, the max Length of hdd password is 32 bytes -// -#define OPAL_PASSWORD_MAX_LENGTH 32 =20 extern VOID *mBuffer; =20 @@ -124,7 +120,7 @@ typedef struct { =20 UINT32 NvmeNamespaceId; =20 - UINT8 Password[32]; + UINT8 Password[OPAL_PASSWORD_MAX_LENG= TH]; UINT8 PasswordLength; =20 UINT32 Length; --=20 2.15.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel