Revision: 14322
http://edk2.svn.sourceforge.net/edk2/?rev=14322&view=rev
Author: vanjeff
Date: 2013-04-28 02:31:36 +0000 (Sun, 28 Apr 2013)
Log Message:
-----------
Sync patches r14276, r14305 and r14317 from main trunk.
1. Fix SMM Variable driver stack GetVariable return INVALID_PARAMETER when
DataSize is bigger than SMM communication buffer.
2. Fix potential overflow for SetVariable interface.
3. 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.
4. Add check to make sure the input VariableName is A Null-terminated string.
5. Use local variable to hold StrSize (VariableName) to avoid duplicated
StrSize calculation.
Revision Links:
--------------
http://edk2.svn.sourceforge.net/edk2/?rev=14276&view=rev
http://edk2.svn.sourceforge.net/edk2/?rev=14305&view=rev
http://edk2.svn.sourceforge.net/edk2/?rev=14317&view=rev
Modified Paths:
--------------
branches/UDK2010.SR1/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
branches/UDK2010.SR1/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
branches/UDK2010.SR1/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
branches/UDK2010.SR1/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
branches/UDK2010.SR1/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/EsalVariableDxeSal/Variable.c
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
Modified:
branches/UDK2010.SR1/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
===================================================================
---
branches/UDK2010.SR1/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
2013-04-28 02:28:40 UTC (rev 14321)
+++
branches/UDK2010.SR1/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -11,7 +11,7 @@
SmmHandlerEntry() will receive untrusted input and do validation.
- Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
which accompanies this distribution. The full text of the license may be
found at
@@ -732,6 +732,33 @@
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
+IsAddressValid (
+ IN UINTN Buffer,
+ IN UINTN Length
+ )
+{
+ if (Buffer > (MAX_ADDRESS - Length)) {
+ //
+ // Overflow happen
+ //
+ return FALSE;
+ }
+ if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {
+ return FALSE;
+ }
+ return TRUE;
+}
+
/**
Thunk service of EFI_SMM_BASE_PROTOCOL.Register().
@@ -1068,7 +1095,7 @@
ASSERT (CommBufferSize != NULL);
if (*CommBufferSize == sizeof (SMMBASE_FUNCTION_DATA) &&
- !IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer,
*CommBufferSize)) {
+ IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {
FunctionData = (SMMBASE_FUNCTION_DATA *)CommBuffer;
switch (FunctionData->Function) {
Modified:
branches/UDK2010.SR1/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
===================================================================
---
branches/UDK2010.SR1/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
2013-04-28 02:28:40 UTC (rev 14321)
+++
branches/UDK2010.SR1/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -16,7 +16,7 @@
SmmPerformanceHandlerEx(), SmmPerformanceHandler() will receive untrusted
input and do basic validation.
-Copyright (c) 2011 - 2012, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
which accompanies this distribution. The full text of the license may be
found at
@@ -478,6 +478,33 @@
}
/**
+ 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
+IsAddressValid (
+ IN UINTN Buffer,
+ IN UINTN Length
+ )
+{
+ if (Buffer > (MAX_ADDRESS - Length)) {
+ //
+ // Overflow happen
+ //
+ return FALSE;
+ }
+ if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {
+ return FALSE;
+ }
+ return TRUE;
+}
+
+/**
Communication service SMI Handler entry.
This SMI handler provides services for the performance wrapper driver.
@@ -527,8 +554,8 @@
return EFI_SUCCESS;
}
- if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer,
*CommBufferSize)) {
- DEBUG ((EFI_D_ERROR, "SMM communcation data buffer is in SMRAM!\n"));
+ if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {
+ DEBUG ((EFI_D_ERROR, "SMM communcation data buffer in SMRAM or
overflow!\n"));
return EFI_SUCCESS;
}
@@ -551,8 +578,8 @@
// Sanity check
//
DataSize = SmmPerfCommData->NumberOfEntries *
sizeof(GAUGE_DATA_ENTRY_EX);
- if (IsAddressInSmram
((EFI_PHYSICAL_ADDRESS)(UINTN)SmmPerfCommData->GaugeDataEx, DataSize)) {
- DEBUG ((EFI_D_ERROR, "SMM Performance Data buffer is in SMRAM!\n"));
+ if (!IsAddressValid ((UINTN)SmmPerfCommData->GaugeDataEx, DataSize)) {
+ DEBUG ((EFI_D_ERROR, "SMM Performance Data buffer in SMRAM or
overflow!\n"));
Status = EFI_ACCESS_DENIED;
break;
}
@@ -628,8 +655,8 @@
return EFI_SUCCESS;
}
- if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer,
*CommBufferSize)) {
- DEBUG ((EFI_D_ERROR, "SMM communcation data buffer is in SMRAM!\n"));
+ if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {
+ DEBUG ((EFI_D_ERROR, "SMM communcation data buffer in SMRAM or
overflow!\n"));
return EFI_SUCCESS;
}
@@ -652,8 +679,8 @@
// Sanity check
//
DataSize = SmmPerfCommData->NumberOfEntries * sizeof(GAUGE_DATA_ENTRY);
- if (IsAddressInSmram
((EFI_PHYSICAL_ADDRESS)(UINTN)SmmPerfCommData->GaugeData, DataSize)) {
- DEBUG ((EFI_D_ERROR, "SMM Performance Data buffer is in SMRAM!\n"));
+ if (!IsAddressValid ((UINTN)SmmPerfCommData->GaugeData, DataSize)) {
+ DEBUG ((EFI_D_ERROR, "SMM Performance Data buffer in SMRAM or
overflow!\n"));
Status = EFI_ACCESS_DENIED;
break;
}
Modified:
branches/UDK2010.SR1/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
===================================================================
---
branches/UDK2010.SR1/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
2013-04-28 02:28:40 UTC (rev 14321)
+++
branches/UDK2010.SR1/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -11,7 +11,7 @@
FpdtSmiHandler() will receive untrusted input and do basic validation.
- Copyright (c) 2011 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
which accompanies this distribution. The full text of the license may be
found at
@@ -206,6 +206,33 @@
}
/**
+ 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;
+}
+
+/**
Communication service SMI Handler entry.
This SMI handler provides services for report SMM boot records.
@@ -252,8 +279,8 @@
return EFI_SUCCESS;
}
- if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer,
*CommBufferSize)) {
- DEBUG ((EFI_D_ERROR, "SMM communication data buffer is in SMRAM!\n"));
+ if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {
+ DEBUG ((EFI_D_ERROR, "SMM communication data buffer in SMRAM or
overflow!\n"));
return EFI_SUCCESS;
}
@@ -276,8 +303,8 @@
// Sanity check
//
SmmCommData->BootRecordSize = mBootRecordSize;
- if (InternalIsAddressInSmram
((EFI_PHYSICAL_ADDRESS)(UINTN)SmmCommData->BootRecordData, mBootRecordSize)) {
- DEBUG ((EFI_D_ERROR, "SMM Data buffer is in SMRAM!\n"));
+ if (!InternalIsAddressValid ((UINTN)SmmCommData->BootRecordData,
mBootRecordSize)) {
+ DEBUG ((EFI_D_ERROR, "SMM Data buffer in SMRAM or overflow!\n"));
Status = EFI_ACCESS_DENIED;
break;
}
Modified:
branches/UDK2010.SR1/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
===================================================================
---
branches/UDK2010.SR1/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
2013-04-28 02:28:40 UTC (rev 14321)
+++
branches/UDK2010.SR1/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -99,7 +99,33 @@
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;
+}
+
/**
Retrive the SMM FVB protocol interface by HANDLE.
@@ -356,8 +382,8 @@
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;
}
Modified:
branches/UDK2010.SR1/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
===================================================================
--- branches/UDK2010.SR1/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
2013-04-28 02:28:40 UTC (rev 14321)
+++ branches/UDK2010.SR1/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -9,7 +9,7 @@
SmmLockBoxHandler(), SmmLockBoxRestore(), SmmLockBoxUpdate(),
SmmLockBoxSave()
will receive untrusted input and do basic validation.
-Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2013, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions
@@ -125,7 +125,7 @@
// Sanity check
//
if (!IsAddressValid ((UINTN)LockBoxParameterSave->Buffer,
(UINTN)LockBoxParameterSave->Length)) {
- DEBUG ((EFI_D_ERROR, "SmmLockBox Save address in SMRAM!\n"));
+ DEBUG ((EFI_D_ERROR, "SmmLockBox Save address in SMRAM or buffer
overflow!\n"));
LockBoxParameterSave->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;
return ;
}
@@ -203,7 +203,7 @@
// Sanity check
//
if (!IsAddressValid ((UINTN)LockBoxParameterUpdate->Buffer,
(UINTN)LockBoxParameterUpdate->Length)) {
- DEBUG ((EFI_D_ERROR, "SmmLockBox Update address in SMRAM!\n"));
+ DEBUG ((EFI_D_ERROR, "SmmLockBox Update address in SMRAM or buffer
overflow!\n"));
LockBoxParameterUpdate->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;
return ;
}
@@ -241,7 +241,7 @@
// Sanity check
//
if (!IsAddressValid ((UINTN)LockBoxParameterRestore->Buffer,
(UINTN)LockBoxParameterRestore->Length)) {
- DEBUG ((EFI_D_ERROR, "SmmLockBox Restore address in SMRAM!\n"));
+ DEBUG ((EFI_D_ERROR, "SmmLockBox Restore address in SMRAM or buffer
overflow!\n"));
LockBoxParameterRestore->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;
return ;
}
@@ -320,7 +320,7 @@
return EFI_SUCCESS;
}
if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {
- DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer in SMRAM!\n"));
+ DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer in SMRAM or overflow!\n"));
return EFI_SUCCESS;
}
Modified:
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
===================================================================
---
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
2013-04-28 02:28:40 UTC (rev 14321)
+++
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -3,7 +3,7 @@
Emulation Variable services operate on the runtime volatile memory.
The nonvolatile variable space doesn't exist.
-Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
which accompanies this distribution. The full text of the license may be
found at
@@ -1397,14 +1397,22 @@
if ((Attributes & (EFI_VARIABLE_RUNTIME_ACCESS |
EFI_VARIABLE_BOOTSERVICE_ACCESS)) == EFI_VARIABLE_RUNTIME_ACCESS) {
return EFI_INVALID_PARAMETER;
}
+
+
+ if ((UINTN)(~0) - DataSize < StrSize(VariableName)){
+ //
+ // Prevent whole variable size overflow
+ //
+ return EFI_INVALID_PARAMETER;
+ }
+
//
// The size of the VariableName, including the Unicode Null in bytes plus
// the DataSize is limited to maximum size of PcdGet32
(PcdMaxHardwareErrorVariableSize)
// bytes for HwErrRec, and PcdGet32 (PcdMaxVariableSize) bytes for the
others.
//
if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) ==
EFI_VARIABLE_HARDWARE_ERROR_RECORD) {
- if ((DataSize > PcdGet32 (PcdMaxHardwareErrorVariableSize)) ||
- (sizeof (VARIABLE_HEADER) + StrSize (VariableName) + DataSize >
PcdGet32 (PcdMaxHardwareErrorVariableSize))) {
+ if (StrSize (VariableName) + DataSize > PcdGet32
(PcdMaxHardwareErrorVariableSize) - sizeof (VARIABLE_HEADER)) {
return EFI_INVALID_PARAMETER;
}
//
@@ -1418,8 +1426,7 @@
// The size of the VariableName, including the Unicode Null in bytes plus
// the DataSize is limited to maximum size of PcdGet32 (PcdMaxVariableSize)
bytes.
//
- if ((DataSize > PcdGet32 (PcdMaxVariableSize)) ||
- (sizeof (VARIABLE_HEADER) + StrSize (VariableName) + DataSize >
PcdGet32 (PcdMaxVariableSize))) {
+ if (StrSize (VariableName) + DataSize > PcdGet32 (PcdMaxVariableSize) -
sizeof (VARIABLE_HEADER)) {
return EFI_INVALID_PARAMETER;
}
}
Modified:
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
===================================================================
--- branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
2013-04-28 02:28:40 UTC (rev 14321)
+++ branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -2110,14 +2110,20 @@
return EFI_INVALID_PARAMETER;
}
+ if ((UINTN)(~0) - DataSize < StrSize(VariableName)){
+ //
+ // Prevent whole variable size overflow
+ //
+ return EFI_INVALID_PARAMETER;
+ }
+
//
// The size of the VariableName, including the Unicode Null in bytes plus
// the DataSize is limited to maximum size of PcdGet32
(PcdMaxHardwareErrorVariableSize)
// bytes for HwErrRec, and PcdGet32 (PcdMaxVariableSize) bytes for the
others.
//
if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) ==
EFI_VARIABLE_HARDWARE_ERROR_RECORD) {
- if ((DataSize > PcdGet32 (PcdMaxHardwareErrorVariableSize)) ||
- (sizeof (VARIABLE_HEADER) + StrSize (VariableName) + DataSize >
PcdGet32 (PcdMaxHardwareErrorVariableSize))) {
+ if ( StrSize (VariableName) + DataSize > PcdGet32
(PcdMaxHardwareErrorVariableSize) - sizeof (VARIABLE_HEADER)) {
return EFI_INVALID_PARAMETER;
}
if (!IsHwErrRecVariable(VariableName, VendorGuid)) {
@@ -2128,8 +2134,7 @@
// The size of the VariableName, including the Unicode Null in bytes plus
// the DataSize is limited to maximum size of PcdGet32
(PcdMaxVariableSize) bytes.
//
- if ((DataSize > PcdGet32 (PcdMaxVariableSize)) ||
- (sizeof (VARIABLE_HEADER) + StrSize (VariableName) + DataSize >
PcdGet32 (PcdMaxVariableSize))) {
+ if (StrSize (VariableName) + DataSize > PcdGet32 (PcdMaxVariableSize) -
sizeof (VARIABLE_HEADER)) {
return EFI_INVALID_PARAMETER;
}
}
Modified:
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
===================================================================
---
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
2013-04-28 02:28:40 UTC (rev 14321)
+++
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -93,7 +93,33 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
//
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;
}
Modified:
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
===================================================================
---
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
2013-04-28 02:28:40 UTC (rev 14321)
+++
branches/UDK2010.SR1/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -189,6 +189,9 @@
EFI_STATUS Status;
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *SmmVariableHeader;
+ UINTN SmmCommBufPayloadSize;
+ UINTN TempDataSize;
+ UINTN VariableNameSize;
if (VariableName == NULL || VendorGuid == NULL || DataSize == NULL) {
return EFI_INVALID_PARAMETER;
@@ -198,13 +201,17 @@
return EFI_INVALID_PARAMETER;
}
- if (*DataSize >= mVariableBufferSize) {
- //
- // DataSize may be near MAX_ADDRESS incorrectly, this can cause the
computed PayLoadSize to
- // overflow to a small value and pass the check in InitCommunicateBuffer().
- // To protect against this vulnerability, return EFI_INVALID_PARAMETER if
DataSize is >= mVariableBufferSize.
- // And there will be further check to ensure the total size is also not >
mVariableBufferSize.
- //
+ //
+ // SMM Communication Buffer max payload size
+ //
+ SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE +
SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);
+ TempDataSize = *DataSize;
+ VariableNameSize = StrSize (VariableName);
+
+ //
+ // If VariableName exceeds SMM payload limit. Return failure
+ //
+ if (VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {
return EFI_INVALID_PARAMETER;
}
@@ -214,7 +221,14 @@
// Init the communicate buffer. The buffer data size is:
// SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE +
PayloadSize.
//
- PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) +
StrSize (VariableName) + *DataSize;
+ if (TempDataSize > SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize) {
+ //
+ // If output data buffer exceed SMM payload limit. Trim output buffer to
SMM payload size
+ //
+ TempDataSize = SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize;
+ }
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) +
VariableNameSize + TempDataSize;
+
Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize,
SMM_VARIABLE_FUNCTION_GET_VARIABLE);
if (EFI_ERROR (Status)) {
goto Done;
@@ -222,8 +236,8 @@
ASSERT (SmmVariableHeader != NULL);
CopyGuid (&SmmVariableHeader->Guid, VendorGuid);
- SmmVariableHeader->DataSize = *DataSize;
- SmmVariableHeader->NameSize = StrSize (VariableName);
+ SmmVariableHeader->DataSize = TempDataSize;
+ SmmVariableHeader->NameSize = VariableNameSize;
if (Attributes == NULL) {
SmmVariableHeader->Attributes = 0;
} else {
@@ -239,7 +253,13 @@
//
// Get data from SMM.
//
- *DataSize = SmmVariableHeader->DataSize;
+ if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
+ //
+ // SMM CommBuffer DataSize can be a trimed value
+ // Only update DataSize when needed
+ //
+ *DataSize = SmmVariableHeader->DataSize;
+ }
if (Attributes != NULL) {
*Attributes = SmmVariableHeader->Attributes;
}
@@ -281,6 +301,8 @@
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *SmmGetNextVariableName;
UINTN SmmCommBufPayloadSize;
+ UINTN OutVariableNameSize;
+ UINTN InVariableNameSize;
if (VariableNameSize == NULL || VariableName == NULL || VendorGuid == NULL) {
return EFI_INVALID_PARAMETER;
@@ -290,11 +312,13 @@
// SMM Communication Buffer max payload size
//
SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE +
SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);
+ OutVariableNameSize = *VariableNameSize;
+ InVariableNameSize = StrSize (VariableName);
//
// If input string exceeds SMM payload limit. Return failure
//
- if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
+ if (InVariableNameSize > SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
return EFI_INVALID_PARAMETER;
}
@@ -304,16 +328,16 @@
// Init the communicate buffer. The buffer data size is:
// SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE +
PayloadSize.
//
- if (*VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
+ if (OutVariableNameSize > SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
//
// If output buffer exceed SMM payload limit. Trim output buffer to SMM
payload size
//
- *VariableNameSize = SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);
+ OutVariableNameSize = SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);
}
//
// Payload should be Guid + NameSize + MAX of Input & Output buffer
//
- PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME,
Name) + MAX (*VariableNameSize, StrSize (VariableName));
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME,
Name) + MAX (OutVariableNameSize, InVariableNameSize);
Status = InitCommunicateBuffer ((VOID **)&SmmGetNextVariableName,
PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);
@@ -325,13 +349,16 @@
//
// SMM comm buffer->NameSize is buffer size for return string
//
- SmmGetNextVariableName->NameSize = *VariableNameSize;
+ SmmGetNextVariableName->NameSize = OutVariableNameSize;
CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid);
//
// Copy whole string
//
- CopyMem (SmmGetNextVariableName->Name, VariableName, StrSize (VariableName));
+ CopyMem (SmmGetNextVariableName->Name, VariableName, InVariableNameSize);
+ if (OutVariableNameSize > InVariableNameSize) {
+ ZeroMem ((UINT8 *) SmmGetNextVariableName->Name + InVariableNameSize,
OutVariableNameSize - InVariableNameSize);
+ }
//
// Send data to SMM
@@ -341,7 +368,13 @@
//
// Get data from SMM.
//
- *VariableNameSize = SmmGetNextVariableName->NameSize;
+ if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
+ //
+ // SMM CommBuffer NameSize can be a trimed value
+ // Only update VariableNameSize when needed
+ //
+ *VariableNameSize = SmmGetNextVariableName->NameSize;
+ }
if (EFI_ERROR (Status)) {
goto Done;
}
@@ -384,6 +417,7 @@
EFI_STATUS Status;
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *SmmVariableHeader;
+ UINTN VariableNameSize;
//
// Check input parameters.
@@ -405,14 +439,22 @@
//
return EFI_INVALID_PARAMETER;
}
+ VariableNameSize = StrSize (VariableName);
+ if ((UINTN)(~0) - VariableNameSize < OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + DataSize) {
+ //
+ // Prevent PayloadSize overflow
+ //
+ return EFI_INVALID_PARAMETER;
+ }
+
AcquireLockOnlyAtBootTime(&mVariableServicesLock);
//
// Init the communicate buffer. The buffer data size is:
// SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE +
PayloadSize.
//
- PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) +
StrSize (VariableName) + DataSize;
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) +
VariableNameSize + DataSize;
Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize,
SMM_VARIABLE_FUNCTION_SET_VARIABLE);
if (EFI_ERROR (Status)) {
goto Done;
@@ -421,7 +463,7 @@
CopyGuid ((EFI_GUID *) &SmmVariableHeader->Guid, VendorGuid);
SmmVariableHeader->DataSize = DataSize;
- SmmVariableHeader->NameSize = StrSize (VariableName);
+ SmmVariableHeader->NameSize = VariableNameSize;
SmmVariableHeader->Attributes = Attributes;
CopyMem (SmmVariableHeader->Name, VariableName, SmmVariableHeader->NameSize);
CopyMem ((UINT8 *) SmmVariableHeader->Name + SmmVariableHeader->NameSize,
Data, DataSize);
Modified:
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/EsalVariableDxeSal/Variable.c
===================================================================
---
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/EsalVariableDxeSal/Variable.c
2013-04-28 02:28:40 UTC (rev 14321)
+++
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/EsalVariableDxeSal/Variable.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -1,7 +1,7 @@
/** @file
The implementation of Extended SAL variable services.
-Copyright (c) 2009 - 2011, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
which accompanies this distribution. The full text of the license may be
found at
@@ -2591,6 +2591,14 @@
PayloadSize = DataSize;
}
+
+ if ((UINTN)(~0) - PayloadSize < StrSize(VariableName)){
+ //
+ // Prevent whole variable size overflow
+ //
+ return EFI_INVALID_PARAMETER;
+ }
+
VariableGlobal = &Global->VariableGlobal[VirtualMode];
Instance = Global->FvbInstance;
@@ -2599,8 +2607,7 @@
// For variable for hardware error record, the size of the VariableName,
including the Unicode Null
// in bytes plus the DataSize is limited to maximum size of
PcdGet32(PcdMaxHardwareErrorVariableSize) bytes.
//
- if ((PayloadSize > PcdGet32(PcdMaxHardwareErrorVariableSize)) ||
- (sizeof (VARIABLE_HEADER) + StrSize (VariableName) + PayloadSize >
PcdGet32(PcdMaxHardwareErrorVariableSize))) {
+ if (StrSize (VariableName) + PayloadSize >
PcdGet32(PcdMaxHardwareErrorVariableSize) - sizeof (VARIABLE_HEADER)) {
return EFI_INVALID_PARAMETER;
}
//
@@ -2616,8 +2623,7 @@
// For variable not for hardware error record, the size of the
VariableName, including the
// Unicode Null in bytes plus the DataSize is limited to maximum size of
PcdGet32(PcdMaxVariableSize) bytes.
//
- if ((PayloadSize > PcdGet32(PcdMaxVariableSize)) ||
- (sizeof (VARIABLE_HEADER) + StrSize (VariableName) + PayloadSize >
PcdGet32(PcdMaxVariableSize))) {
+ if (StrSize (VariableName) + PayloadSize > PcdGet32(PcdMaxVariableSize) -
sizeof (VARIABLE_HEADER)) {
return EFI_INVALID_PARAMETER;
}
}
Modified:
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
===================================================================
---
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
2013-04-28 02:28:40 UTC (rev 14321)
+++
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -2318,14 +2318,20 @@
PayloadSize = DataSize;
}
+ if ((UINTN)(~0) - PayloadSize < StrSize(VariableName)){
+ //
+ // Prevent whole variable size overflow
+ //
+ return EFI_INVALID_PARAMETER;
+ }
+
//
// The size of the VariableName, including the Unicode Null in bytes plus
// the DataSize is limited to maximum size of PcdGet32
(PcdMaxHardwareErrorVariableSize)
// bytes for HwErrRec, and PcdGet32 (PcdMaxVariableSize) bytes for the
others.
//
if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) ==
EFI_VARIABLE_HARDWARE_ERROR_RECORD) {
- if ((PayloadSize > PcdGet32 (PcdMaxHardwareErrorVariableSize)) ||
- (sizeof (VARIABLE_HEADER) + StrSize (VariableName) + PayloadSize >
PcdGet32 (PcdMaxHardwareErrorVariableSize))) {
+ if (StrSize (VariableName) + PayloadSize > PcdGet32
(PcdMaxHardwareErrorVariableSize) - sizeof (VARIABLE_HEADER)) {
return EFI_INVALID_PARAMETER;
}
if (!IsHwErrRecVariable(VariableName, VendorGuid)) {
@@ -2336,8 +2342,7 @@
// The size of the VariableName, including the Unicode Null in bytes plus
// the DataSize is limited to maximum size of PcdGet32
(PcdMaxVariableSize) bytes.
//
- if ((PayloadSize > PcdGet32 (PcdMaxVariableSize)) ||
- (sizeof (VARIABLE_HEADER) + StrSize (VariableName) + PayloadSize >
PcdGet32 (PcdMaxVariableSize))) {
+ if (StrSize (VariableName) + PayloadSize > PcdGet32 (PcdMaxVariableSize) -
sizeof (VARIABLE_HEADER)) {
return EFI_INVALID_PARAMETER;
}
}
Modified:
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
===================================================================
---
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
2013-04-28 02:28:40 UTC (rev 14321)
+++
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -93,7 +93,33 @@
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.
@@ -423,6 +449,7 @@
SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO *QueryVariableInfo;
VARIABLE_INFO_ENTRY *VariableInfo;
UINTN InfoSize;
+ UINTN NameBufferSize;
//
// If input is invalid, stop processing this SMI
@@ -435,8 +462,8 @@
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;
}
@@ -445,6 +472,14 @@
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;
@@ -457,6 +492,14 @@
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,
@@ -468,6 +511,13 @@
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;
//
@@ -479,6 +529,15 @@
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,
@@ -488,6 +547,14 @@
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;
@@ -501,6 +568,14 @@
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,
@@ -555,7 +630,7 @@
//
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;
}
Modified:
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
===================================================================
---
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
2013-04-28 02:28:40 UTC (rev 14321)
+++
branches/UDK2010.SR1/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
2013-04-28 02:31:36 UTC (rev 14322)
@@ -205,6 +205,9 @@
EFI_STATUS Status;
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *SmmVariableHeader;
+ UINTN SmmCommBufPayloadSize;
+ UINTN TempDataSize;
+ UINTN VariableNameSize;
if (VariableName == NULL || VendorGuid == NULL || DataSize == NULL) {
return EFI_INVALID_PARAMETER;
@@ -214,13 +217,17 @@
return EFI_INVALID_PARAMETER;
}
- if (*DataSize >= mVariableBufferSize) {
- //
- // DataSize may be near MAX_ADDRESS incorrectly, this can cause the
computed PayLoadSize to
- // overflow to a small value and pass the check in InitCommunicateBuffer().
- // To protect against this vulnerability, return EFI_INVALID_PARAMETER if
DataSize is >= mVariableBufferSize.
- // And there will be further check to ensure the total size is also not >
mVariableBufferSize.
- //
+ //
+ // SMM Communication Buffer max payload size
+ //
+ SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE +
SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);
+ TempDataSize = *DataSize;
+ VariableNameSize = StrSize (VariableName);
+
+ //
+ // If VariableName exceeds SMM payload limit. Return failure
+ //
+ if (VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {
return EFI_INVALID_PARAMETER;
}
@@ -230,7 +237,14 @@
// Init the communicate buffer. The buffer data size is:
// SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE +
PayloadSize.
//
- PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) +
StrSize (VariableName) + *DataSize;
+ if (TempDataSize > SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize) {
+ //
+ // If output data buffer exceed SMM payload limit. Trim output buffer to
SMM payload size
+ //
+ TempDataSize = SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize;
+ }
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) +
VariableNameSize + TempDataSize;
+
Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize,
SMM_VARIABLE_FUNCTION_GET_VARIABLE);
if (EFI_ERROR (Status)) {
goto Done;
@@ -238,8 +252,8 @@
ASSERT (SmmVariableHeader != NULL);
CopyGuid (&SmmVariableHeader->Guid, VendorGuid);
- SmmVariableHeader->DataSize = *DataSize;
- SmmVariableHeader->NameSize = StrSize (VariableName);
+ SmmVariableHeader->DataSize = TempDataSize;
+ SmmVariableHeader->NameSize = VariableNameSize;
if (Attributes == NULL) {
SmmVariableHeader->Attributes = 0;
} else {
@@ -255,7 +269,13 @@
//
// Get data from SMM.
//
- *DataSize = SmmVariableHeader->DataSize;
+ if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
+ //
+ // SMM CommBuffer DataSize can be a trimed value
+ // Only update DataSize when needed
+ //
+ *DataSize = SmmVariableHeader->DataSize;
+ }
if (Attributes != NULL) {
*Attributes = SmmVariableHeader->Attributes;
}
@@ -297,6 +317,8 @@
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *SmmGetNextVariableName;
UINTN SmmCommBufPayloadSize;
+ UINTN OutVariableNameSize;
+ UINTN InVariableNameSize;
if (VariableNameSize == NULL || VariableName == NULL || VendorGuid == NULL) {
return EFI_INVALID_PARAMETER;
@@ -306,11 +328,13 @@
// SMM Communication Buffer max payload size
//
SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE +
SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);
+ OutVariableNameSize = *VariableNameSize;
+ InVariableNameSize = StrSize (VariableName);
//
// If input string exceeds SMM payload limit. Return failure
//
- if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
+ if (InVariableNameSize > SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
return EFI_INVALID_PARAMETER;
}
@@ -320,16 +344,16 @@
// Init the communicate buffer. The buffer data size is:
// SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE +
PayloadSize.
//
- if (*VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
+ if (OutVariableNameSize > SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
//
// If output buffer exceed SMM payload limit. Trim output buffer to SMM
payload size
//
- *VariableNameSize = SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);
+ OutVariableNameSize = SmmCommBufPayloadSize - OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);
}
//
// Payload should be Guid + NameSize + MAX of Input & Output buffer
//
- PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME,
Name) + MAX (*VariableNameSize, StrSize (VariableName));
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME,
Name) + MAX (OutVariableNameSize, InVariableNameSize);
Status = InitCommunicateBuffer ((VOID **)&SmmGetNextVariableName,
PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);
if (EFI_ERROR (Status)) {
@@ -340,13 +364,16 @@
//
// SMM comm buffer->NameSize is buffer size for return string
//
- SmmGetNextVariableName->NameSize = *VariableNameSize;
+ SmmGetNextVariableName->NameSize = OutVariableNameSize;
CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid);
//
// Copy whole string
//
- CopyMem (SmmGetNextVariableName->Name, VariableName, StrSize (VariableName));
+ CopyMem (SmmGetNextVariableName->Name, VariableName, InVariableNameSize);
+ if (OutVariableNameSize > InVariableNameSize) {
+ ZeroMem ((UINT8 *) SmmGetNextVariableName->Name + InVariableNameSize,
OutVariableNameSize - InVariableNameSize);
+ }
//
// Send data to SMM
@@ -356,7 +383,13 @@
//
// Get data from SMM.
//
- *VariableNameSize = SmmGetNextVariableName->NameSize;
+ if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
+ //
+ // SMM CommBuffer NameSize can be a trimed value
+ // Only update VariableNameSize when needed
+ //
+ *VariableNameSize = SmmGetNextVariableName->NameSize;
+ }
if (EFI_ERROR (Status)) {
goto Done;
}
@@ -402,6 +435,7 @@
EFI_STATUS Status;
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *SmmVariableHeader;
+ UINTN VariableNameSize;
//
// Check input parameters.
@@ -423,14 +457,22 @@
//
return EFI_INVALID_PARAMETER;
}
+ VariableNameSize = StrSize (VariableName);
+ if ((UINTN)(~0) - VariableNameSize < OFFSET_OF
(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + DataSize) {
+ //
+ // Prevent PayloadSize overflow
+ //
+ return EFI_INVALID_PARAMETER;
+ }
+
AcquireLockOnlyAtBootTime(&mVariableServicesLock);
-
+
//
// Init the communicate buffer. The buffer data size is:
// SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE +
PayloadSize.
//
- PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) +
StrSize (VariableName) + DataSize;
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) +
VariableNameSize + DataSize;
Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize,
SMM_VARIABLE_FUNCTION_SET_VARIABLE);
if (EFI_ERROR (Status)) {
goto Done;
@@ -439,7 +481,7 @@
CopyGuid ((EFI_GUID *) &SmmVariableHeader->Guid, VendorGuid);
SmmVariableHeader->DataSize = DataSize;
- SmmVariableHeader->NameSize = StrSize (VariableName);
+ SmmVariableHeader->NameSize = VariableNameSize;
SmmVariableHeader->Attributes = Attributes;
CopyMem (SmmVariableHeader->Name, VariableName, SmmVariableHeader->NameSize);
CopyMem ((UINT8 *) SmmVariableHeader->Name + SmmVariableHeader->NameSize,
Data, DataSize);
This was sent by the SourceForge.net collaborative development platform, the
world's largest Open Source development site.
------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_apr
_______________________________________________
edk2-commits mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-commits
