Add in ASSERT to check out-of-bound and possible dereference of NULL pointers.

git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@7391 6f19259b-4bc3-4df7-8a09-765794883524

4 files changed, 26 insertions, 5 deletions

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c b/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
index ed77094540..fc687540be 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
@@ -773,7 +773,7 @@ IfrMatch (
     }

 

     String[Index] = GetToken (Value.Value.string, FormSet->HiiHandle);

-    if (String== NULL) {

+    if (String [Index] == NULL) {

       Status = EFI_NOT_FOUND;

       goto Done;

     }

@@ -1111,7 +1111,7 @@ IfrSpan (
     }

 

     String[Index] = GetToken (Value.Value.string, FormSet->HiiHandle);

-    if (String== NULL) {

+    if (String [Index] == NULL) {

       Status = EFI_NOT_FOUND;

       goto Done;

     }

@@ -1299,7 +1299,9 @@ CompareHiiValue (
 

 

 /**

-  Evaluate the result of a HII expression

+  Evaluate the result of a HII expression.

+

+  If Expression is NULL, then ASSERT.

 

   @param  FormSet                FormSet associated with this expression.

   @param  Form                   Form associated with this expression.

@@ -1341,6 +1343,7 @@ EvaluateExpression (
   //

   ResetExpressionStack ();

 

+  ASSERT (Expression != NULL);

   Expression->Result.Type = EFI_IFR_TYPE_OTHER;

 

   Link = GetFirstNode (&Expression->OpCodeListHead);

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
index 7a09b32b73..60992a31dd 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
@@ -1059,7 +1059,6 @@ ParseOpCodes (
           //

           // Evaluate DisableIf expression

           //

-          ASSERT (CurrentExpression != NULL);

           Status = EvaluateExpression (FormSet, CurrentForm, CurrentExpression);

           if (EFI_ERROR (Status)) {

             return Status;

@@ -1193,6 +1192,8 @@ ParseOpCodes (
     //

     case EFI_IFR_SUBTITLE_OP:

       CurrentStatement = CreateStatement (OpCodeData, FormSet, CurrentForm);

+      ASSERT (CurrentStatement != NULL);

+      

       CurrentStatement->Flags = ((EFI_IFR_SUBTITLE *) OpCodeData)->Flags;

 

       if (Scope != 0) {

@@ -1202,6 +1203,7 @@ ParseOpCodes (
 

     case EFI_IFR_TEXT_OP:

       CurrentStatement = CreateStatement (OpCodeData, FormSet, CurrentForm);

+      ASSERT (CurrentStatement != NULL);

 

       CopyMem (&CurrentStatement->TextTwo, &((EFI_IFR_TEXT *) OpCodeData)->TextTwo, sizeof (EFI_STRING_ID));

       break;

@@ -1211,6 +1213,7 @@ ParseOpCodes (
     //

     case EFI_IFR_ACTION_OP:

       CurrentStatement = CreateQuestion (OpCodeData, FormSet, CurrentForm);

+      ASSERT (CurrentStatement != NULL);

 

       if (OpCodeLength == sizeof (EFI_IFR_ACTION_1)) {

         //

@@ -1548,6 +1551,12 @@ ParseOpCodes (
         //

         // If used for a question, then the question will be read-only

         //

+        //

+        // Make sure CurrentStatement is not NULL.

+        // If it is NULL, 1) ParseOpCodes functions may parse the IFR wrongly. Or 2) the IFR

+        // file is wrongly generated by tools such as VFR Compiler. There may be a bug in VFR Compiler.

+        //

+        ASSERT (CurrentStatement != NULL);

         CurrentStatement->ValueExpression = CurrentExpression;

       }

       break;

@@ -1585,6 +1594,12 @@ ParseOpCodes (
         break;

 

       default:

+        //

+        // Make sure CurrentStatement is not NULL.

+        // If it is NULL, 1) ParseOpCodes functions may parse the IFR wrongly. Or 2) the IFR

+        // file is wrongly generated by tools such as VFR Compiler.

+        //

+        ASSERT (CurrentStatement != NULL);

         ImageId = &CurrentStatement->ImageId;

         break;

       }

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/InputHandler.c b/MdeModulePkg/Universal/SetupBrowserDxe/InputHandler.c
index d122aec569..1bf7704343 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/InputHandler.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/InputHandler.c
@@ -387,7 +387,7 @@ GetNumericInput (
 

     InputText[0] = LEFT_NUMERIC_DELIMITER;

     SetUnicodeMem (InputText + 1, InputWidth, L' ');

-    ASSERT (InputWidth < MAX_NUMERIC_INPUT_WIDTH); 

+    ASSERT (InputWidth + 2 < MAX_NUMERIC_INPUT_WIDTH); 

     InputText[InputWidth + 1] = RIGHT_NUMERIC_DELIMITER;

     InputText[InputWidth + 2] = L'\0';

 

@@ -649,6 +649,7 @@ EnterCarriageReturn:
         }

 

         Count++;

+        ASSERT (Count < (sizeof (PreviousNumber) / sizeof (PreviousNumber[0])));

         PreviousNumber[Count] = EditValue;

 

         PrintCharAt (Column, Row, Key.UnicodeChar);

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c b/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c
index 8ac531f09c..447e6a2718 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c
@@ -896,6 +896,8 @@ ProcessHelpString (
       AllocateSize += 0x10;

       OldIndexArray  = IndexArray;

       IndexArray = AllocatePool (AllocateSize * sizeof (UINTN) * 3);

+      ASSERT (IndexArray != NULL);

+      

       CopyMem (IndexArray, OldIndexArray, LineCount * sizeof (UINTN) * 3);

       FreePool (OldIndexArray);

     }