Skip to content

Commit

Permalink
MdeModulePkg/SecurityPkg Variable: Add boundary check for while (IsVa…
Browse files Browse the repository at this point in the history 
…lidVariableHeader (Variable)).

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <[email protected]>
Reviewed-by: Jiewen Yao <[email protected]>

git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16280 6f19259b-4bc3-4df7-8a09-765794883524
  • Loading branch information
@lzeng14
lzeng14 authored and lzeng14 committed Oct 31, 2014
1 parent a75cf43 commit 6ebffb6
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 56 deletions.
22 changes: 20 additions & 2 deletions MdeModulePkg/Universal/Variable/Pei/Variable.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Implement ReadOnly Variable Services required by PEIM and install
PEI ReadOnly Varaiable2 PPI. These services operates the non volatile storage space.
Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2006 - 2014, 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 @@ -547,14 +547,25 @@ GetVariableHeader (
EFI_HOB_GUID_TYPE *GuidHob;
UINTN PartialHeaderSize;

if (Variable == NULL) {
return FALSE;
}

//
// First assume variable header pointed by Variable is consecutive.
//
*VariableHeader = Variable;

if ((Variable != NULL) && (StoreInfo->FtwLastWriteData != NULL)) {
if (StoreInfo->FtwLastWriteData != NULL) {
TargetAddress = StoreInfo->FtwLastWriteData->TargetAddress;
SpareAddress = StoreInfo->FtwLastWriteData->SpareAddress;
if (((UINTN) Variable > (UINTN) SpareAddress) &&
(((UINTN) Variable - (UINTN) SpareAddress + (UINTN) TargetAddress) >= (UINTN) GetEndPointer (StoreInfo->VariableStoreHeader))) {
//
// Reach the end of variable store.
//
return FALSE;
}
if (((UINTN) Variable < (UINTN) TargetAddress) && (((UINTN) Variable + sizeof (VARIABLE_HEADER)) > (UINTN) TargetAddress)) {
//
// Variable header pointed by Variable is inconsecutive,
Expand All @@ -576,6 +587,13 @@ GetVariableHeader (
CopyMem ((UINT8 *) *VariableHeader + PartialHeaderSize, (UINT8 *) (UINTN) SpareAddress, sizeof (VARIABLE_HEADER) - PartialHeaderSize);
}
}
} else {
if (Variable >= GetEndPointer (StoreInfo->VariableStoreHeader)) {
//
// Reach the end of variable store.
//
return FALSE;
}
}

return IsValidVariableHeader (*VariableHeader);
Expand Down
46 changes: 22 additions & 24 deletions MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,18 +190,24 @@ UpdateVariableInfo (
This code checks if variable header is valid or not.
@param Variable Pointer to the Variable Header.
@param Variable Pointer to the Variable Header.
@param VariableStoreEnd Pointer to the Variable Store End.
@retval TRUE Variable header is valid.
@retval FALSE Variable header is not valid.
@retval TRUE Variable header is valid.
@retval FALSE Variable header is not valid.
**/
BOOLEAN
IsValidVariableHeader (
IN VARIABLE_HEADER *Variable
IN VARIABLE_HEADER *Variable,
IN VARIABLE_HEADER *VariableStoreEnd
)
{
if (Variable == NULL || Variable->StartId != VARIABLE_DATA) {
if ((Variable == NULL) || (Variable >= VariableStoreEnd) || (Variable->StartId != VARIABLE_DATA)) {
//
// Variable is NULL or has reached the end of variable store,
// or the StartId is not correct.
//
return FALSE;
}

Expand Down Expand Up @@ -499,10 +505,6 @@ GetNextVariablePtr (
{
UINTN Value;

if (!IsValidVariableHeader (Variable)) {
return NULL;
}

Value = (UINTN) GetVariableDataPtr (Variable);
Value += DataSizeOfVariable (Variable);
Value += GET_PAD_SIZE (DataSizeOfVariable (Variable));
Expand Down Expand Up @@ -622,7 +624,7 @@ Reclaim (
Variable = GetStartPointer (VariableStoreHeader);
MaximumBufferSize = sizeof (VARIABLE_STORE_HEADER);

while (IsValidVariableHeader (Variable)) {
while (IsValidVariableHeader (Variable, GetEndPointer (VariableStoreHeader))) {
NextVariable = GetNextVariablePtr (Variable);
if ((Variable->State == VAR_ADDED || Variable->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED)) &&
Variable != UpdatingVariable &&
Expand Down Expand Up @@ -672,7 +674,7 @@ Reclaim (
// Reinstall all ADDED variables as long as they are not identical to Updating Variable.
//
Variable = GetStartPointer (VariableStoreHeader);
while (IsValidVariableHeader (Variable)) {
while (IsValidVariableHeader (Variable, GetEndPointer (VariableStoreHeader))) {
NextVariable = GetNextVariablePtr (Variable);
if (Variable != UpdatingVariable && Variable->State == VAR_ADDED) {
VariableSize = (UINTN) NextVariable - (UINTN) Variable;
Expand All @@ -691,7 +693,7 @@ Reclaim (
// Reinstall all in delete transition variables.
//
Variable = GetStartPointer (VariableStoreHeader);
while (IsValidVariableHeader (Variable)) {
while (IsValidVariableHeader (Variable, GetEndPointer (VariableStoreHeader))) {
NextVariable = GetNextVariablePtr (Variable);
if (Variable != UpdatingVariable && Variable != UpdatingInDeletedTransition && Variable->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED)) {

Expand All @@ -703,7 +705,7 @@ Reclaim (

FoundAdded = FALSE;
AddedVariable = GetStartPointer ((VARIABLE_STORE_HEADER *) ValidBuffer);
while (IsValidVariableHeader (AddedVariable)) {
while (IsValidVariableHeader (AddedVariable, GetEndPointer ((VARIABLE_STORE_HEADER *) ValidBuffer))) {
NextAddedVariable = GetNextVariablePtr (AddedVariable);
NameSize = NameSizeOfVariable (AddedVariable);
if (CompareGuid (&AddedVariable->VendorGuid, &Variable->VendorGuid) &&
Expand Down Expand Up @@ -795,7 +797,7 @@ Reclaim (
mVariableModuleGlobal->CommonVariableTotalSize = CommonVariableTotalSize;
} else {
NextVariable = GetStartPointer ((VARIABLE_STORE_HEADER *)(UINTN)VariableBase);
while (IsValidVariableHeader (NextVariable)) {
while (IsValidVariableHeader (NextVariable, GetEndPointer ((VARIABLE_STORE_HEADER *)(UINTN)VariableBase))) {
VariableSize = NextVariable->NameSize + NextVariable->DataSize + sizeof (VARIABLE_HEADER);
if ((Variable->Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) == EFI_VARIABLE_HARDWARE_ERROR_RECORD) {
mVariableModuleGlobal->HwErrVariableTotalSize += HEADER_ALIGN (VariableSize);
Expand Down Expand Up @@ -853,7 +855,7 @@ FindVariableEx (
InDeletedVariable = NULL;

for ( PtrTrack->CurrPtr = PtrTrack->StartPtr
; (PtrTrack->CurrPtr < PtrTrack->EndPtr) && IsValidVariableHeader (PtrTrack->CurrPtr)
; IsValidVariableHeader (PtrTrack->CurrPtr, PtrTrack->EndPtr)
; PtrTrack->CurrPtr = GetNextVariablePtr (PtrTrack->CurrPtr)
) {
if (PtrTrack->CurrPtr->State == VAR_ADDED ||
Expand Down Expand Up @@ -2408,10 +2410,7 @@ VariableServiceGetNextVariableName (
//
// Switch from Volatile to HOB, to Non-Volatile.
//
while ((Variable.CurrPtr >= Variable.EndPtr) ||
(Variable.CurrPtr == NULL) ||
!IsValidVariableHeader (Variable.CurrPtr)
) {
while (!IsValidVariableHeader (Variable.CurrPtr, Variable.EndPtr)) {
//
// Find current storage index
//
Expand Down Expand Up @@ -2617,8 +2616,7 @@ VariableServiceSetVariable (
// Parse non-volatile variable data and get last variable offset.
//
NextVariable = GetStartPointer ((VARIABLE_STORE_HEADER *) (UINTN) Point);
while ((NextVariable < GetEndPointer ((VARIABLE_STORE_HEADER *) (UINTN) Point))
&& IsValidVariableHeader (NextVariable)) {
while (IsValidVariableHeader (NextVariable, GetEndPointer ((VARIABLE_STORE_HEADER *) (UINTN) Point))) {
NextVariable = GetNextVariablePtr (NextVariable);
}
mVariableModuleGlobal->NonVolatileLastVariableOffset = (UINTN) NextVariable - (UINTN) Point;
Expand Down Expand Up @@ -2765,7 +2763,7 @@ VariableServiceQueryVariableInfoInternal (
//
// Now walk through the related variable store.
//
while ((Variable < GetEndPointer (VariableStoreHeader)) && IsValidVariableHeader (Variable)) {
while (IsValidVariableHeader (Variable, GetEndPointer (VariableStoreHeader))) {
NextVariable = GetNextVariablePtr (Variable);
VariableSize = (UINT64) (UINTN) NextVariable - (UINT64) (UINTN) Variable;

Expand Down Expand Up @@ -3063,7 +3061,7 @@ InitNonVolatileVariableStore (
// Parse non-volatile variable data and get last variable offset.
//
NextVariable = GetStartPointer ((VARIABLE_STORE_HEADER *)(UINTN)VariableStoreBase);
while (IsValidVariableHeader (NextVariable)) {
while (IsValidVariableHeader (NextVariable, GetEndPointer ((VARIABLE_STORE_HEADER *)(UINTN)VariableStoreBase))) {
VariableSize = NextVariable->NameSize + NextVariable->DataSize + sizeof (VARIABLE_HEADER);
if ((NextVariable->Attributes & (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_HARDWARE_ERROR_RECORD)) == (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_HARDWARE_ERROR_RECORD)) {
mVariableModuleGlobal->HwErrVariableTotalSize += HEADER_ALIGN (VariableSize);
Expand Down Expand Up @@ -3109,7 +3107,7 @@ FlushHobVariableToFlash (
//
mVariableModuleGlobal->VariableGlobal.HobVariableBase = 0;
for ( Variable = GetStartPointer (VariableStoreHeader)
; (Variable < GetEndPointer (VariableStoreHeader) && IsValidVariableHeader (Variable))
; IsValidVariableHeader (Variable, GetEndPointer (VariableStoreHeader))
; Variable = GetNextVariablePtr (Variable)
) {
if (Variable->State != VAR_ADDED) {
Expand Down
22 changes: 20 additions & 2 deletions SecurityPkg/VariableAuthenticated/Pei/Variable.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
ReadOnly Varaiable2 PPI. These services operates the non-volatile
storage space.
Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2009 - 2014, 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 @@ -546,14 +546,25 @@ GetVariableHeader (
EFI_HOB_GUID_TYPE *GuidHob;
UINTN PartialHeaderSize;

if (Variable == NULL) {
return FALSE;
}

//
// First assume variable header pointed by Variable is consecutive.
//
*VariableHeader = Variable;

if ((Variable != NULL) && (StoreInfo->FtwLastWriteData != NULL)) {
if (StoreInfo->FtwLastWriteData != NULL) {
TargetAddress = StoreInfo->FtwLastWriteData->TargetAddress;
SpareAddress = StoreInfo->FtwLastWriteData->SpareAddress;
if (((UINTN) Variable > (UINTN) SpareAddress) &&
(((UINTN) Variable - (UINTN) SpareAddress + (UINTN) TargetAddress) >= (UINTN) GetEndPointer (StoreInfo->VariableStoreHeader))) {
//
// Reach the end of variable store.
//
return FALSE;
}
if (((UINTN) Variable < (UINTN) TargetAddress) && (((UINTN) Variable + sizeof (VARIABLE_HEADER)) > (UINTN) TargetAddress)) {
//
// Variable header pointed by Variable is inconsecutive,
Expand All @@ -575,6 +586,13 @@ GetVariableHeader (
CopyMem ((UINT8 *) *VariableHeader + PartialHeaderSize, (UINT8 *) (UINTN) SpareAddress, sizeof (VARIABLE_HEADER) - PartialHeaderSize);
}
}
} else {
if (Variable >= GetEndPointer (StoreInfo->VariableStoreHeader)) {
//
// Reach the end of variable store.
//
return FALSE;
}
}

return IsValidVariableHeader (*VariableHeader);
Expand Down
Loading

0 comments on commit 6ebffb6

Please sign in to comment.