From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 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) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False 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 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,426,1526367600"; d="scan'208";a="77387615" 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 From: Eric Dong To: edk2-devel@lists.01.org Cc: Yao Jiewen , Wu Hao 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: [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: , X-List-Received-Date: Tue, 31 Jul 2018 05:16:00 -0000 Current code not check the CommunicationBuffer size before use it. Attacker can read beyond the end of the (untrusted) commbuffer into controlled memory. Attacker can get access outside of valid SMM memory regions. This patch add check before use it. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Eric Dong Cc: Yao Jiewen Cc: Wu Hao --- .../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/SecurityPkg/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 +#define OPAL_PASSWORD_MAX_LENGTH 32 #pragma pack(1) @@ -76,7 +77,7 @@ typedef struct { typedef struct { LIST_ENTRY Link; - UINT8 Password[32]; + UINT8 Password[OPAL_PASSWORD_MAX_LENGTH]; UINT8 PasswordLength; EFI_DEVICE_PATH_PROTOCOL OpalDevicePath; diff --git a/SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.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. #include "OpalPasswordSupportNotify.h" -#define OPAL_PASSWORD_MAX_LENGTH 32 - LIST_ENTRY mDeviceList = INITIALIZE_LIST_HEAD_VARIABLE (mDeviceList); BOOLEAN gInSmm = FALSE; EFI_GUID gOpalPasswordNotifyProtocolGuid = OPAL_PASSWORD_NOTIFY_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; if (CommBuffer == NULL || CommBufferSize == NULL) { return EFI_SUCCESS; } + Status = EFI_SUCCESS; + DeviceBuffer = NULL; + TempCommBufferSize = *CommBufferSize; if (TempCommBufferSize < OFFSET_OF (OPAL_SMM_COMMUNICATE_HEADER, Data)) { return EFI_SUCCESS; } - - Status = EFI_SUCCESS; - SmmFunctionHeader = (OPAL_SMM_COMMUNICATE_HEADER *)CommBuffer; - - DevicePath = &((OPAL_COMM_DEVICE_LIST*)(SmmFunctionHeader->Data))->OpalDevicePath; - PasswordLength = ((OPAL_COMM_DEVICE_LIST*)(SmmFunctionHeader->Data))->PasswordLength; - NewPassword = ((OPAL_COMM_DEVICE_LIST*)(SmmFunctionHeader->Data))->Password; + SmmFunctionHeader = (OPAL_SMM_COMMUNICATE_HEADER *)CommBuffer; switch (SmmFunctionHeader->Function) { case SMM_FUNCTION_SET_OPAL_PASSWORD: - if (OpalPasswordIsFullZero (NewPassword) || PasswordLength == 0) { - Status = EFI_INVALID_PARAMETER; - goto EXIT; - } + if (TempCommBufferSize <= OFFSET_OF (OPAL_SMM_COMMUNICATE_HEADER, Data) + OFFSET_OF (OPAL_COMM_DEVICE_LIST, OpalDevicePath)) { + return EFI_SUCCESS; + } + + DeviceBuffer = AllocateCopyPool (TempCommBufferSize - OFFSET_OF (OPAL_SMM_COMMUNICATE_HEADER, Data), SmmFunctionHeader->Data); + if (DeviceBuffer == NULL) { + Status = EFI_OUT_OF_RESOURCES; + goto EXIT; + } - Status = OpalSavePasswordToSmm (DevicePath, NewPassword, PasswordLength); + // + // Validate the DevicePath. + // + RemainedDevicePathSize = TempCommBufferSize - OFFSET_OF (OPAL_SMM_COMMUNICATE_HEADER, Data) + - OFFSET_OF (OPAL_COMM_DEVICE_LIST, OpalDevicePath); + if (!IsDevicePathValid(&DeviceBuffer->OpalDevicePath, RemainedDevicePathSize) || + (RemainedDevicePathSize != GetDevicePathSize (&DeviceBuffer->OpalDevicePath))) { + Status = EFI_SUCCESS; + goto EXIT; + } + + if (OpalPasswordIsFullZero (DeviceBuffer->Password) || + DeviceBuffer->PasswordLength == 0 || + DeviceBuffer->PasswordLength > OPAL_PASSWORD_MAX_LENGTH) { + Status = EFI_INVALID_PARAMETER; + goto EXIT; + } + + Status = OpalSavePasswordToSmm (&DeviceBuffer->OpalDevicePath, DeviceBuffer->Password, DeviceBuffer->PasswordLength); break; default: @@ -701,6 +718,10 @@ SmmOpalPasswordHandler ( EXIT: SmmFunctionHeader->ReturnStatus = Status; + if (DeviceBuffer != 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 be trigged. diff --git a/SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportNotify.h b/SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportNotify.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; typedef struct { - UINT8 Password[32]; + UINT8 Password[OPAL_PASSWORD_MAX_LENGTH]; UINT8 PasswordLength; EFI_DEVICE_PATH_PROTOCOL OpalDevicePath; diff --git a/SecurityPkg/Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.h b/SecurityPkg/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, EITHER 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 extern VOID *mBuffer; @@ -124,7 +120,7 @@ typedef struct { UINT32 NvmeNamespaceId; - UINT8 Password[32]; + UINT8 Password[OPAL_PASSWORD_MAX_LENGTH]; UINT8 PasswordLength; UINT32 Length; -- 2.15.0.windows.1