Skip to content

Commit

Permalink
Sync patches r14276, r14305 and r14317 from main trunk.
Browse files Browse the repository at this point in the history 
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.

git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/branches/UDK2010.SR1@14322 6f19259b-4bc3-4df7-8a09-765794883524
  • Loading branch information
vanjeff committed Apr 28, 2013
1 parent 6d90839 commit 700ab68
Show file tree
Hide file tree
Showing 13 changed files with 452 additions and 88 deletions.
31 changes: 29 additions & 2 deletions EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -732,6 +732,33 @@ IsAddressInSmram (
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().
Expand Down Expand Up @@ -1068,7 +1095,7 @@ SmmHandlerEntry (
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) {
Expand Down
45 changes: 36 additions & 9 deletions MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -477,6 +477,33 @@ IsAddressInSmram (
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;
}

/**
Communication service SMI Handler entry.
Expand Down Expand Up @@ -527,8 +554,8 @@ SmmPerformanceHandlerEx (
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;
}

Expand All @@ -551,8 +578,8 @@ SmmPerformanceHandlerEx (
// 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;
}
Expand Down Expand Up @@ -628,8 +655,8 @@ SmmPerformanceHandler (
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;
}

Expand All @@ -652,8 +679,8 @@ SmmPerformanceHandler (
// 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;
}
Expand Down
37 changes: 32 additions & 5 deletions MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -205,6 +205,33 @@ 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;
}

/**
Communication service SMI Handler entry.
Expand Down Expand Up @@ -252,8 +279,8 @@ FpdtSmiHandler (
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;
}

Expand All @@ -276,8 +303,8 @@ FpdtSmiHandler (
// 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;
}
Expand Down
30 changes: 28 additions & 2 deletions MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,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;
}

/**
Retrive the SMM FVB protocol interface by HANDLE.
Expand Down Expand Up @@ -356,8 +382,8 @@ SmmFaultTolerantWriteHandler (
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;
}

Expand Down
10 changes: 5 additions & 5 deletions MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -125,7 +125,7 @@ SmmLockBoxSave (
// 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 ;
}
Expand Down Expand Up @@ -203,7 +203,7 @@ SmmLockBoxUpdate (
// 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 ;
}
Expand Down Expand Up @@ -241,7 +241,7 @@ SmmLockBoxRestore (
// 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 ;
}
Expand Down Expand Up @@ -320,7 +320,7 @@ SmmLockBoxHandler (
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;
}

Expand Down
17 changes: 12 additions & 5 deletions MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1397,14 +1397,22 @@ EmuSetVariable (
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;
}
//
Expand All @@ -1418,8 +1426,7 @@ EmuSetVariable (
// 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;
}
}
Expand Down
13 changes: 9 additions & 4 deletions MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2110,14 +2110,20 @@ VariableServiceSetVariable (
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)) {
Expand All @@ -2128,8 +2134,7 @@ VariableServiceSetVariable (
// 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;
}
}
Expand Down
Loading

0 comments on commit 700ab68

Please sign in to comment.