From 6ebffb67c8eca68cf5eb36bd308b305ab84fdd99 Mon Sep 17 00:00:00 2001 From: Star Zeng Date: Fri, 31 Oct 2014 10:26:54 +0000 Subject: MdeModulePkg/SecurityPkg Variable: Add boundary check for while (IsValidVariableHeader (Variable)). Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Star Zeng Reviewed-by: Jiewen Yao git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16280 6f19259b-4bc3-4df7-8a09-765794883524 --- .../Universal/Variable/RuntimeDxe/Variable.c | 46 +++++++++++----------- 1 file changed, 22 insertions(+), 24 deletions(-) (limited to 'MdeModulePkg/Universal/Variable/RuntimeDxe') diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c index fc7fbab5d6..c66bdbf9fd 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c @@ -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; } @@ -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)); @@ -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 && @@ -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; @@ -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)) { @@ -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) && @@ -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); @@ -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 || @@ -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 // @@ -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; @@ -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; @@ -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); @@ -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) { -- cgit v1.2.3