Fix the TOCTOU issue of CommBufferSize itself for SMM communicate handler input.

Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>

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

6 files changed, 53 insertions, 26 deletions

diff --git a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
index 2bfd62a2b9..f95079bd27 100644
--- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
+++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
@@ -543,6 +543,7 @@ SmmPerformanceHandlerEx (
   GAUGE_DATA_ENTRY_EX       *GaugeDataEx;

   UINTN                     NumberOfEntries;

   UINTN                     LogEntryKey;

+  UINTN                     TempCommBufferSize;

 

   GaugeEntryExArray = NULL;

 

@@ -553,11 +554,13 @@ SmmPerformanceHandlerEx (
     return EFI_SUCCESS;

   }

 

-  if(*CommBufferSize < sizeof (SMM_PERF_COMMUNICATE_EX)) {

+  TempCommBufferSize = *CommBufferSize;

+

+  if(TempCommBufferSize < sizeof (SMM_PERF_COMMUNICATE_EX)) {

     return EFI_SUCCESS;

   }

 

-  if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {

+  if (!IsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {

     DEBUG ((EFI_D_ERROR, "SmmPerformanceHandlerEx: SMM communcation data buffer in SMRAM or overflow!\n"));

     return EFI_SUCCESS;

   }

@@ -649,7 +652,8 @@ SmmPerformanceHandler (
   GAUGE_DATA_ENTRY      *GaugeData;

   UINTN                 NumberOfEntries;

   UINTN                 LogEntryKey;

-  

+  UINTN                 TempCommBufferSize;

+

   GaugeEntryExArray = NULL;

 

   //

@@ -659,11 +663,13 @@ SmmPerformanceHandler (
     return EFI_SUCCESS;

   }

 

-  if(*CommBufferSize < sizeof (SMM_PERF_COMMUNICATE)) {

+  TempCommBufferSize = *CommBufferSize;

+

+  if(TempCommBufferSize < sizeof (SMM_PERF_COMMUNICATE)) {

     return EFI_SUCCESS;

   }

 

-  if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {

+  if (!IsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {

     DEBUG ((EFI_D_ERROR, "SmmPerformanceHandler: SMM communcation data buffer in SMRAM or overflow!\n"));

     return EFI_SUCCESS;

   }

diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
index f3472e26f3..9c5fd4db85 100644
--- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
+++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
@@ -268,6 +268,7 @@ FpdtSmiHandler (
   SMM_BOOT_RECORD_COMMUNICATE  *SmmCommData;

   UINTN                        BootRecordSize;

   VOID                         *BootRecordData;

+  UINTN                        TempCommBufferSize;

 

   //

   // If input is invalid, stop processing this SMI

@@ -276,11 +277,13 @@ FpdtSmiHandler (
     return EFI_SUCCESS;

   }

 

-  if(*CommBufferSize < sizeof (SMM_BOOT_RECORD_COMMUNICATE)) {

+  TempCommBufferSize = *CommBufferSize;

+

+  if(TempCommBufferSize < sizeof (SMM_BOOT_RECORD_COMMUNICATE)) {

     return EFI_SUCCESS;

   }

   

-  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {

+  if (!InternalIsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {

     DEBUG ((EFI_D_ERROR, "FpdtSmiHandler: SMM communication data buffer in SMRAM or overflow!\n"));

     return EFI_SUCCESS;

   }

diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
index 2580d478a3..2b3a63081d 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
@@ -372,7 +372,7 @@ SmmFaultTolerantWriteHandler (
   UINTN                                            CommBufferPayloadSize;

   UINTN                                            PrivateDataSize;

   UINTN                                            Length;

-

+  UINTN                                            TempCommBufferSize;

 

   //

   // If input is invalid, stop processing this SMI

@@ -381,13 +381,15 @@ SmmFaultTolerantWriteHandler (
     return EFI_SUCCESS;

   }

 

-  if (*CommBufferSize < SMM_FTW_COMMUNICATE_HEADER_SIZE) {

+  TempCommBufferSize = *CommBufferSize;

+

+  if (TempCommBufferSize < SMM_FTW_COMMUNICATE_HEADER_SIZE) {

     DEBUG ((EFI_D_ERROR, "SmmFtwHandler: SMM communication buffer size invalid!\n"));

     return EFI_SUCCESS;

   }

-  CommBufferPayloadSize = *CommBufferSize - SMM_FTW_COMMUNICATE_HEADER_SIZE;

+  CommBufferPayloadSize = TempCommBufferSize - SMM_FTW_COMMUNICATE_HEADER_SIZE;

 

-  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {

+  if (!InternalIsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {

     DEBUG ((EFI_D_ERROR, "SmmFtwHandler: SMM communication buffer in SMRAM or overflow!\n"));

     return EFI_SUCCESS;

   }

diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
index 4cb88106ee..ad4b2645cb 100644
--- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
+++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
@@ -321,17 +321,27 @@ SmmLockBoxHandler (
   )

 {

   EFI_SMM_LOCK_BOX_PARAMETER_HEADER *LockBoxParameterHeader;

+  UINTN                             TempCommBufferSize;

 

   DEBUG ((EFI_D_ERROR, "SmmLockBox SmmLockBoxHandler Enter\n"));

 

   //

+  // If input is invalid, stop processing this SMI

+  //

+  if (CommBuffer == NULL || CommBufferSize == NULL) {

+    return EFI_SUCCESS;

+  }

+

+  TempCommBufferSize = *CommBufferSize;

+

+  //

   // Sanity check

   //

-  if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_HEADER)) {

+  if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_HEADER)) {

     DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size invalid!\n"));

     return EFI_SUCCESS;

   }

-  if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {

+  if (!IsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {

     DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer in SMRAM or overflow!\n"));

     return EFI_SUCCESS;

   }

@@ -346,35 +356,35 @@ SmmLockBoxHandler (
 

   switch (LockBoxParameterHeader->Command) {

   case EFI_SMM_LOCK_BOX_COMMAND_SAVE:

-    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)) {

+    if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)) {

       DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for SAVE invalid!\n"));

       break;

     }

     SmmLockBoxSave ((EFI_SMM_LOCK_BOX_PARAMETER_SAVE *)(UINTN)LockBoxParameterHeader);

     break;

   case EFI_SMM_LOCK_BOX_COMMAND_UPDATE:

-    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)) {

+    if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)) {

       DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for UPDATE invalid!\n"));

       break;

     }

     SmmLockBoxUpdate ((EFI_SMM_LOCK_BOX_PARAMETER_UPDATE *)(UINTN)LockBoxParameterHeader);

     break;

   case EFI_SMM_LOCK_BOX_COMMAND_RESTORE:

-    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)) {

+    if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)) {

       DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for RESTORE invalid!\n"));

       break;

     }

     SmmLockBoxRestore ((EFI_SMM_LOCK_BOX_PARAMETER_RESTORE *)(UINTN)LockBoxParameterHeader);

     break;

   case EFI_SMM_LOCK_BOX_COMMAND_SET_ATTRIBUTES:

-    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)) {

+    if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)) {

       DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for SET_ATTRIBUTES invalid!\n"));

       break;

     }

     SmmLockBoxSetAttributes ((EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES *)(UINTN)LockBoxParameterHeader);

     break;

   case EFI_SMM_LOCK_BOX_COMMAND_RESTORE_ALL_IN_PLACE:

-    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)) {

+    if (TempCommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)) {

       DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for RESTORE_ALL_IN_PLACE invalid!\n"));

       break;

     }

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index aea9d4bcfe..1ffa74e6cc 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -501,6 +501,7 @@ SmmVariableHandler (
   UINTN                                            InfoSize;

   UINTN                                            NameBufferSize;

   UINTN                                            CommBufferPayloadSize;

+  UINTN                                            TempCommBufferSize;

 

   //

   // If input is invalid, stop processing this SMI

@@ -509,17 +510,19 @@ SmmVariableHandler (
     return EFI_SUCCESS;

   }

 

-  if (*CommBufferSize < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) {

+  TempCommBufferSize = *CommBufferSize;

+

+  if (TempCommBufferSize < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) {

     DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer size invalid!\n"));

     return EFI_SUCCESS;

   }

-  CommBufferPayloadSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;

+  CommBufferPayloadSize = TempCommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;

   if (CommBufferPayloadSize > mVariableBufferPayloadSize) {

     DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer payload size invalid!\n"));

     return EFI_SUCCESS;

   }

 

-  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {

+  if (!InternalIsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {

     DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer in SMRAM or overflow!\n"));

     return EFI_SUCCESS;

   }

@@ -699,7 +702,7 @@ SmmVariableHandler (
 

     case SMM_VARIABLE_FUNCTION_GET_STATISTICS:

       VariableInfo = (VARIABLE_INFO_ENTRY *) SmmVariableFunctionHeader->Data;

-      InfoSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;

+      InfoSize = TempCommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;

 

       //

       // Do not need to check SmmVariableFunctionHeader->Data in SMRAM here. 

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
index cf866cecba..0be4f254d7 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
@@ -506,6 +506,7 @@ SmmVariableHandler (
   UINTN                                            InfoSize;

   UINTN                                            NameBufferSize;

   UINTN                                            CommBufferPayloadSize;

+  UINTN                                            TempCommBufferSize;

 

   //

   // If input is invalid, stop processing this SMI

@@ -514,17 +515,19 @@ SmmVariableHandler (
     return EFI_SUCCESS;

   }

 

-  if (*CommBufferSize < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) {

+  TempCommBufferSize = *CommBufferSize;

+

+  if (TempCommBufferSize < SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) {

     DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer size invalid!\n"));

     return EFI_SUCCESS;

   }

-  CommBufferPayloadSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;

+  CommBufferPayloadSize = TempCommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;

   if (CommBufferPayloadSize > mVariableBufferPayloadSize) {

     DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer payload size invalid!\n"));

     return EFI_SUCCESS;

   }

 

-  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {

+  if (!InternalIsAddressValid ((UINTN)CommBuffer, TempCommBufferSize)) {

     DEBUG ((EFI_D_ERROR, "SmmVariableHandler: SMM communication buffer in SMRAM or overflow!\n"));

     return EFI_SUCCESS;

   }

@@ -705,7 +708,7 @@ SmmVariableHandler (
 

     case SMM_VARIABLE_FUNCTION_GET_STATISTICS:

       VariableInfo = (VARIABLE_INFO_ENTRY *) SmmVariableFunctionHeader->Data;

-      InfoSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;

+      InfoSize = TempCommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;

 

       //

       // Do not need to check SmmVariableFunctionHeader->Data in SMRAM here.