1. Use the check IsAddressValid() to prevent SMM communication buffer overflow in SmmVariable, FtwSmm, FpdtSmm, SmmCorePerformance and SmmBaseHelper, and add check to prevent InfoSize overflows in SmmVariableHandler.
2. Refine the debug message. 3. Add check to make sure the input VariableName is A Null-terminated string. 4. Use local variable to hold StrSize (VariableName) to avoid duplicated StrSize calculation. Signed-off-by: Star Zeng <star.zeng@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com> Reviewed-by: Chao Zhang <chao.b.zhang@intel.com> git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@14317 6f19259b-4bc3-4df7-8a09-765794883524
This commit is contained in:
@@ -93,6 +93,32 @@ InternalIsAddressInSmram (
|
||||
return FALSE;
|
||||
}
|
||||
|
||||
/**
|
||||
This function check if the address refered by Buffer and Length is valid.
|
||||
|
||||
@param Buffer the buffer address to be checked.
|
||||
@param Length the buffer length to be checked.
|
||||
|
||||
@retval TRUE this address is valid.
|
||||
@retval FALSE this address is NOT valid.
|
||||
**/
|
||||
BOOLEAN
|
||||
InternalIsAddressValid (
|
||||
IN UINTN Buffer,
|
||||
IN UINTN Length
|
||||
)
|
||||
{
|
||||
if (Buffer > (MAX_ADDRESS - Length)) {
|
||||
//
|
||||
// Overflow happen
|
||||
//
|
||||
return FALSE;
|
||||
}
|
||||
if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {
|
||||
return FALSE;
|
||||
}
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
/**
|
||||
Initializes a basic mutual exclusion lock.
|
||||
@@ -418,6 +444,7 @@ SmmVariableHandler (
|
||||
SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO *QueryVariableInfo;
|
||||
VARIABLE_INFO_ENTRY *VariableInfo;
|
||||
UINTN InfoSize;
|
||||
UINTN NameBufferSize;
|
||||
|
||||
//
|
||||
// If input is invalid, stop processing this SMI
|
||||
@@ -430,8 +457,8 @@ SmmVariableHandler (
|
||||
return EFI_SUCCESS;
|
||||
}
|
||||
|
||||
if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {
|
||||
DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));
|
||||
if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {
|
||||
DEBUG ((EFI_D_ERROR, "SMM communication buffer in SMRAM or overflow!\n"));
|
||||
return EFI_SUCCESS;
|
||||
}
|
||||
|
||||
@@ -439,6 +466,14 @@ SmmVariableHandler (
|
||||
switch (SmmVariableFunctionHeader->Function) {
|
||||
case SMM_VARIABLE_FUNCTION_GET_VARIABLE:
|
||||
SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;
|
||||
if (((UINTN)(~0) - SmmVariableHeader->DataSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) ||
|
||||
((UINTN)(~0) - SmmVariableHeader->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + SmmVariableHeader->DataSize)) {
|
||||
//
|
||||
// Prevent InfoSize overflow happen
|
||||
//
|
||||
Status = EFI_ACCESS_DENIED;
|
||||
goto EXIT;
|
||||
}
|
||||
InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)
|
||||
+ SmmVariableHeader->DataSize + SmmVariableHeader->NameSize;
|
||||
|
||||
@@ -451,6 +486,14 @@ SmmVariableHandler (
|
||||
goto EXIT;
|
||||
}
|
||||
|
||||
if (SmmVariableHeader->NameSize < sizeof (CHAR16) || SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != L'\0') {
|
||||
//
|
||||
// Make sure VariableName is A Null-terminated string.
|
||||
//
|
||||
Status = EFI_ACCESS_DENIED;
|
||||
goto EXIT;
|
||||
}
|
||||
|
||||
Status = VariableServiceGetVariable (
|
||||
SmmVariableHeader->Name,
|
||||
&SmmVariableHeader->Guid,
|
||||
@@ -462,6 +505,13 @@ SmmVariableHandler (
|
||||
|
||||
case SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME:
|
||||
GetNextVariableName = (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) SmmVariableFunctionHeader->Data;
|
||||
if ((UINTN)(~0) - GetNextVariableName->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
|
||||
//
|
||||
// Prevent InfoSize overflow happen
|
||||
//
|
||||
Status = EFI_ACCESS_DENIED;
|
||||
goto EXIT;
|
||||
}
|
||||
InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + GetNextVariableName->NameSize;
|
||||
|
||||
//
|
||||
@@ -473,6 +523,15 @@ SmmVariableHandler (
|
||||
goto EXIT;
|
||||
}
|
||||
|
||||
NameBufferSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE - OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);
|
||||
if (NameBufferSize < sizeof (CHAR16) || GetNextVariableName->Name[NameBufferSize/sizeof (CHAR16) - 1] != L'\0') {
|
||||
//
|
||||
// Make sure input VariableName is A Null-terminated string.
|
||||
//
|
||||
Status = EFI_ACCESS_DENIED;
|
||||
goto EXIT;
|
||||
}
|
||||
|
||||
Status = VariableServiceGetNextVariableName (
|
||||
&GetNextVariableName->NameSize,
|
||||
GetNextVariableName->Name,
|
||||
@@ -482,6 +541,14 @@ SmmVariableHandler (
|
||||
|
||||
case SMM_VARIABLE_FUNCTION_SET_VARIABLE:
|
||||
SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;
|
||||
if (((UINTN)(~0) - SmmVariableHeader->DataSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) ||
|
||||
((UINTN)(~0) - SmmVariableHeader->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + SmmVariableHeader->DataSize)) {
|
||||
//
|
||||
// Prevent InfoSize overflow happen
|
||||
//
|
||||
Status = EFI_ACCESS_DENIED;
|
||||
goto EXIT;
|
||||
}
|
||||
InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)
|
||||
+ SmmVariableHeader->DataSize + SmmVariableHeader->NameSize;
|
||||
|
||||
@@ -495,6 +562,14 @@ SmmVariableHandler (
|
||||
goto EXIT;
|
||||
}
|
||||
|
||||
if (SmmVariableHeader->NameSize < sizeof (CHAR16) || SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != L'\0') {
|
||||
//
|
||||
// Make sure VariableName is A Null-terminated string.
|
||||
//
|
||||
Status = EFI_ACCESS_DENIED;
|
||||
goto EXIT;
|
||||
}
|
||||
|
||||
Status = VariableServiceSetVariable (
|
||||
SmmVariableHeader->Name,
|
||||
&SmmVariableHeader->Guid,
|
||||
@@ -549,7 +624,7 @@ SmmVariableHandler (
|
||||
//
|
||||
|
||||
if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBufferSize, sizeof(UINTN))) {
|
||||
DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));
|
||||
DEBUG ((EFI_D_ERROR, "SMM communication buffer in SMRAM!\n"));
|
||||
Status = EFI_ACCESS_DENIED;
|
||||
goto EXIT;
|
||||
}
|
||||
|
Reference in New Issue
Block a user