MdeModulePkg/SecurityPkg Variable: Add boundary check for while (IsValidVariableHeader (Variable)).

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>

git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16280 6f19259b-4bc3-4df7-8a09-765794883524

1 files changed, 22 insertions, 24 deletions

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) {