1. Use the check IsAddressValid() to prevent SMM communication buffer overflow in...

[mirror_edk2.git] / MdeModulePkg / Universal / Variable / RuntimeDxe / VariableSmm.c

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index 7541f6ae1d2606144c1b4778f48469df162a4637..f8e6bd58828ae592e205d4fdeafe94961cee767b 100644 (file)
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -93,6 +93,32 @@ InternalIsAddressInSmram (
   return FALSE;\r
 }\r
 \r
+/**\r
+  This function check if the address refered by Buffer and Length is valid.\r
+\r
+  @param Buffer  the buffer address to be checked.\r
+  @param Length  the buffer length to be checked.\r
+\r
+  @retval TRUE  this address is valid.\r
+  @retval FALSE this address is NOT valid.\r
+**/\r
+BOOLEAN\r
+InternalIsAddressValid (\r
+  IN UINTN                 Buffer,\r
+  IN UINTN                 Length\r
+  )\r
+{\r
+  if (Buffer > (MAX_ADDRESS - Length)) {\r
+    //\r
+    // Overflow happen\r
+    //\r
+    return FALSE;\r
+  }\r
+  if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {\r
+    return FALSE;\r
+  }\r
+  return TRUE;\r
+}\r
 \r
 /**\r
   Initializes a basic mutual exclusion lock.\r
@@ -418,6 +444,7 @@ SmmVariableHandler (
   SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO     *QueryVariableInfo;\r
   VARIABLE_INFO_ENTRY                              *VariableInfo;\r
   UINTN                                            InfoSize;\r
+  UINTN                                            NameBufferSize;\r
 \r
   //\r
   // If input is invalid, stop processing this SMI\r
@@ -430,8 +457,8 @@ SmmVariableHandler (
     return EFI_SUCCESS;\r
   }\r
 \r
-  if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBuffer, *CommBufferSize)) {\r
-    DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));\r
+  if (!InternalIsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+    DEBUG ((EFI_D_ERROR, "SMM communication buffer in SMRAM or overflow!\n"));\r
     return EFI_SUCCESS;\r
   }\r
 \r
@@ -439,6 +466,14 @@ SmmVariableHandler (
   switch (SmmVariableFunctionHeader->Function) {\r
     case SMM_VARIABLE_FUNCTION_GET_VARIABLE:\r
       SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;\r
+      if (((UINTN)(~0) - SmmVariableHeader->DataSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) ||\r
+         ((UINTN)(~0) - SmmVariableHeader->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + SmmVariableHeader->DataSize)) {\r
+        //\r
+        // Prevent InfoSize overflow happen\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
       InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) \r
                  + SmmVariableHeader->DataSize + SmmVariableHeader->NameSize;\r
 \r
@@ -451,6 +486,14 @@ SmmVariableHandler (
         goto EXIT;\r
       }\r
 \r
+      if (SmmVariableHeader->NameSize < sizeof (CHAR16) || SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != L'\0') {\r
+        //\r
+        // Make sure VariableName is A Null-terminated string.\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
+\r
       Status = VariableServiceGetVariable (\r
                  SmmVariableHeader->Name,\r
                  &SmmVariableHeader->Guid,\r
@@ -462,6 +505,13 @@ SmmVariableHandler (
       \r
     case SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME:\r
       GetNextVariableName = (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) SmmVariableFunctionHeader->Data;\r
+      if ((UINTN)(~0) - GetNextVariableName->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {\r
+        //\r
+        // Prevent InfoSize overflow happen\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
       InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + GetNextVariableName->NameSize;\r
 \r
       //\r
@@ -473,6 +523,15 @@ SmmVariableHandler (
         goto EXIT;\r
       }\r
 \r
+      NameBufferSize = *CommBufferSize - SMM_VARIABLE_COMMUNICATE_HEADER_SIZE -  OFFSET_OF(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);\r
+      if (NameBufferSize < sizeof (CHAR16) || GetNextVariableName->Name[NameBufferSize/sizeof (CHAR16) - 1] != L'\0') {\r
+        //\r
+        // Make sure input VariableName is A Null-terminated string.\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
+\r
       Status = VariableServiceGetNextVariableName (\r
                  &GetNextVariableName->NameSize,\r
                  GetNextVariableName->Name,\r
@@ -482,6 +541,14 @@ SmmVariableHandler (
       \r
     case SMM_VARIABLE_FUNCTION_SET_VARIABLE:\r
       SmmVariableHeader = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *) SmmVariableFunctionHeader->Data;\r
+      if (((UINTN)(~0) - SmmVariableHeader->DataSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) ||\r
+         ((UINTN)(~0) - SmmVariableHeader->NameSize < OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + SmmVariableHeader->DataSize)) {\r
+        //\r
+        // Prevent InfoSize overflow happen\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
       InfoSize = OFFSET_OF(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)\r
                  + SmmVariableHeader->DataSize + SmmVariableHeader->NameSize;\r
 \r
@@ -495,6 +562,14 @@ SmmVariableHandler (
         goto EXIT;\r
       }\r
 \r
+      if (SmmVariableHeader->NameSize < sizeof (CHAR16) || SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != L'\0') {\r
+        //\r
+        // Make sure VariableName is A Null-terminated string.\r
+        //\r
+        Status = EFI_ACCESS_DENIED;\r
+        goto EXIT;\r
+      }\r
+\r
       Status = VariableServiceSetVariable (\r
                  SmmVariableHeader->Name,\r
                  &SmmVariableHeader->Guid,\r
@@ -549,7 +624,7 @@ SmmVariableHandler (
       //\r
      \r
       if (InternalIsAddressInSmram ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBufferSize, sizeof(UINTN))) {\r
-        DEBUG ((EFI_D_ERROR, "SMM communication buffer size is in SMRAM!\n"));\r
+        DEBUG ((EFI_D_ERROR, "SMM communication buffer in SMRAM!\n"));\r
         Status = EFI_ACCESS_DENIED;\r
         goto EXIT;\r
       }  \r