MdeModulePkg/PiSmmCore: SmmEntryPoint underflow (CVE-2021-38578)

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3387

Added use of SafeIntLib to validate values are not causing overflows or
underflows in user controlled values when calculating buffer sizes.

Signed-off-by: Miki Demeter <miki.demeter@intel.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
This commit is contained in:
Miki Demeter 2022-10-27 16:20:54 -07:00 committed by mergify[bot]
parent c46204e25f
commit cab1f02565
5 changed files with 59 additions and 14 deletions

View File

@ -610,6 +610,7 @@ SmmEndOfS3ResumeHandler (
@param[in] Size2 Size of Buff2
@retval TRUE Buffers overlap in memory.
@retval TRUE Math error. Prevents potential math over and underflows.
@retval FALSE Buffer doesn't overlap.
**/
@ -621,11 +622,24 @@ InternalIsBufferOverlapped (
IN UINTN Size2
)
{
UINTN End1;
UINTN End2;
BOOLEAN IsOverUnderflow1;
BOOLEAN IsOverUnderflow2;
// Check for over or underflow
IsOverUnderflow1 = EFI_ERROR (SafeUintnAdd ((UINTN)Buff1, Size1, &End1));
IsOverUnderflow2 = EFI_ERROR (SafeUintnAdd ((UINTN)Buff2, Size2, &End2));
if (IsOverUnderflow1 || IsOverUnderflow2) {
return TRUE;
}
//
// If buff1's end is less than the start of buff2, then it's ok.
// Also, if buff1's start is beyond buff2's end, then it's ok.
//
if (((Buff1 + Size1) <= Buff2) || (Buff1 >= (Buff2 + Size2))) {
if ((End1 <= (UINTN)Buff2) || ((UINTN)Buff1 >= End2)) {
return FALSE;
}
@ -651,6 +665,7 @@ SmmEntryPoint (
EFI_SMM_COMMUNICATE_HEADER *CommunicateHeader;
BOOLEAN InLegacyBoot;
BOOLEAN IsOverlapped;
BOOLEAN IsOverUnderflow;
VOID *CommunicationBuffer;
UINTN BufferSize;
@ -699,23 +714,31 @@ SmmEntryPoint (
(UINT8 *)gSmmCorePrivate,
sizeof (*gSmmCorePrivate)
);
if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferSize) || IsOverlapped) {
//
// Check for over or underflows
//
IsOverUnderflow = EFI_ERROR (SafeUintnSub (BufferSize, OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data), &BufferSize));
if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferSize) ||
IsOverlapped || IsOverUnderflow)
{
//
// If CommunicationBuffer is not in valid address scope,
// or there is overlap between gSmmCorePrivate and CommunicationBuffer,
// or there is over or underflow,
// return EFI_INVALID_PARAMETER
//
gSmmCorePrivate->CommunicationBuffer = NULL;
gSmmCorePrivate->ReturnStatus = EFI_ACCESS_DENIED;
} else {
CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *)CommunicationBuffer;
BufferSize -= OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data);
Status = SmiManage (
&CommunicateHeader->HeaderGuid,
NULL,
CommunicateHeader->Data,
&BufferSize
);
// BufferSize was updated by the SafeUintnSub() call above.
Status = SmiManage (
&CommunicateHeader->HeaderGuid,
NULL,
CommunicateHeader->Data,
&BufferSize
);
//
// Update CommunicationBuffer, BufferSize and ReturnStatus
// Communicate service finished, reset the pointer to CommBuffer to NULL

View File

@ -54,6 +54,7 @@
#include <Library/PerformanceLib.h>
#include <Library/HobLib.h>
#include <Library/SmmMemLib.h>
#include <Library/SafeIntLib.h>
#include "PiSmmCorePrivateData.h"
#include "HeapGuard.h"

View File

@ -60,6 +60,7 @@
PerformanceLib
HobLib
SmmMemLib
SafeIntLib
[Protocols]
gEfiDxeSmmReadyToLockProtocolGuid ## UNDEFINED # SmiHandlerRegister

View File

@ -34,8 +34,8 @@
#include <Library/UefiRuntimeLib.h>
#include <Library/PcdLib.h>
#include <Library/ReportStatusCodeLib.h>
#include "PiSmmCorePrivateData.h"
#include <Library/SafeIntLib.h>
#define SMRAM_CAPABILITIES (EFI_MEMORY_WB | EFI_MEMORY_UC)
@ -1354,6 +1354,7 @@ SmmSplitSmramEntry (
@param[in] ReservedRangeToCompare Pointer to EFI_SMM_RESERVED_SMRAM_REGION to compare.
@retval TRUE There is overlap.
@retval TRUE Math error.
@retval FALSE There is no overlap.
**/
@ -1363,11 +1364,29 @@ SmmIsSmramOverlap (
IN EFI_SMM_RESERVED_SMRAM_REGION *ReservedRangeToCompare
)
{
UINT64 RangeToCompareEnd;
UINT64 ReservedRangeToCompareEnd;
UINT64 RangeToCompareEnd;
UINT64 ReservedRangeToCompareEnd;
BOOLEAN IsOverUnderflow1;
BOOLEAN IsOverUnderflow2;
RangeToCompareEnd = RangeToCompare->CpuStart + RangeToCompare->PhysicalSize;
ReservedRangeToCompareEnd = ReservedRangeToCompare->SmramReservedStart + ReservedRangeToCompare->SmramReservedSize;
// Check for over or underflow.
IsOverUnderflow1 = EFI_ERROR (
SafeUint64Add (
(UINT64)RangeToCompare->CpuStart,
RangeToCompare->PhysicalSize,
&RangeToCompareEnd
)
);
IsOverUnderflow2 = EFI_ERROR (
SafeUint64Add (
(UINT64)ReservedRangeToCompare->SmramReservedStart,
ReservedRangeToCompare->SmramReservedSize,
&ReservedRangeToCompareEnd
)
);
if (IsOverUnderflow1 || IsOverUnderflow2) {
return TRUE;
}
if ((RangeToCompare->CpuStart >= ReservedRangeToCompare->SmramReservedStart) &&
(RangeToCompare->CpuStart < ReservedRangeToCompareEnd))

View File

@ -46,6 +46,7 @@
DxeServicesLib
PcdLib
ReportStatusCodeLib
SafeIntLib
[Protocols]
gEfiSmmBase2ProtocolGuid ## PRODUCES