Fix memory overflow & VariableSize check issue for SetVariable append write.

Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
Reviewed-by  : Fu Siyuan  <siyuan.fu@intel.com>
Reviewed-by  : Dong Guo   <guo.dong@intel.com>

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

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
index 6f8808a756f67d8c036db67e436465f2a1b80221..440ede9144db4cecde60dc730b50352deffe9875 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
@@ -192,7 +192,7 @@ AutenticatedVariableServiceInitialize (
   //\r
   // Reserved runtime buffer for "Append" operation in virtual mode.\r
   //\r
-  mStorageArea  = AllocateRuntimePool (PcdGet32 (PcdMaxVariableSize));\r
+  mStorageArea  = AllocateRuntimePool (MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32 (PcdMaxHardwareErrorVariableSize)));\r
   if (mStorageArea == NULL) {\r
     return EFI_OUT_OF_RESOURCES;\r
   }\r
@@ -1316,20 +1316,24 @@ ProcessVariable (
   will be appended to the original EFI_SIGNATURE_LIST, duplicate EFI_SIGNATURE_DATA\r
   will be ignored.\r
 \r
-  @param[in, out]  Data            Pointer to original EFI_SIGNATURE_LIST.\r
-  @param[in]       DataSize        Size of Data buffer.\r
-  @param[in]       NewData         Pointer to new EFI_SIGNATURE_LIST to be appended.\r
-  @param[in]       NewDataSize     Size of NewData buffer.\r
+  @param[in, out]  Data              Pointer to original EFI_SIGNATURE_LIST.\r
+  @param[in]       DataSize          Size of Data buffer.\r
+  @param[in]       FreeBufSize       Size of free data buffer \r
+  @param[in]       NewData           Pointer to new EFI_SIGNATURE_LIST to be appended.\r
+  @param[in]       NewDataSize       Size of NewData buffer.\r
+  @param[out]      MergedBufSize     Size of the merged buffer\r
 \r
-  @return Size of the merged buffer.\r
+  @return EFI_BUFFER_TOO_SMALL if input Data buffer overflowed\r
 \r
 **/\r
-UINTN\r
+EFI_STATUS\r
 AppendSignatureList (\r
   IN  OUT VOID            *Data,\r
   IN  UINTN               DataSize,\r
+  IN  UINTN               FreeBufSize,\r
   IN  VOID                *NewData,\r
-  IN  UINTN               NewDataSize\r
+  IN  UINTN               NewDataSize,\r
+  OUT UINTN               *MergedBufSize\r
   )\r
 {\r
   EFI_SIGNATURE_LIST  *CertList;\r
@@ -1388,15 +1392,25 @@ AppendSignatureList (
         // New EFI_SIGNATURE_DATA, append it.\r
         //\r
         if (CopiedCount == 0) {\r
+          if (FreeBufSize < sizeof (EFI_SIGNATURE_LIST) + NewCertList->SignatureHeaderSize) {\r
+            return EFI_BUFFER_TOO_SMALL;\r
+          }\r
+\r
           //\r
           // Copy EFI_SIGNATURE_LIST header for only once.\r
           //\r
+\r
           CopyMem (Tail, NewCertList, sizeof (EFI_SIGNATURE_LIST) + NewCertList->SignatureHeaderSize);\r
           Tail = Tail + sizeof (EFI_SIGNATURE_LIST) + NewCertList->SignatureHeaderSize;\r
+          FreeBufSize -= sizeof (EFI_SIGNATURE_LIST) + NewCertList->SignatureHeaderSize;\r
         }\r
 \r
+        if (FreeBufSize < NewCertList->SignatureSize) {\r
+          return EFI_BUFFER_TOO_SMALL;\r
+        }\r
         CopyMem (Tail, NewCert, NewCertList->SignatureSize);\r
         Tail += NewCertList->SignatureSize;\r
+        FreeBufSize -= NewCertList->SignatureSize;\r
         CopiedCount++;\r
       }\r
 \r
@@ -1416,7 +1430,8 @@ AppendSignatureList (
     NewCertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) NewCertList + NewCertList->SignatureListSize);\r
   }\r
 \r
-  return (Tail - (UINT8 *) Data);\r
+  *MergedBufSize = (Tail - (UINT8 *) Data);\r
+  return EFI_SUCCESS;\r
 }\r
 \r
 /**\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
index 510030d705120ccce51af1ea5146616d2aa1150c..1fd687b03364c87b1bf063223d09f65780807700 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
@@ -2,7 +2,7 @@
   The internal header file includes the common header files, defines\r
   internal structure and functions used by AuthService module.\r
 \r
-Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.<BR>\r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r
 which accompanies this distribution.  The full text of the license may be found at\r
@@ -249,20 +249,24 @@ ProcessVarWithKek (
   will be appended to the original EFI_SIGNATURE_LIST, duplicate EFI_SIGNATURE_DATA\r
   will be ignored.\r
 \r
-  @param[in, out]  Data            Pointer to original EFI_SIGNATURE_LIST.\r
-  @param[in]       DataSize        Size of Data buffer.\r
-  @param[in]       NewData         Pointer to new EFI_SIGNATURE_LIST to be appended.\r
-  @param[in]       NewDataSize     Size of NewData buffer.\r
+  @param[in, out]  Data             Pointer to original EFI_SIGNATURE_LIST.\r
+  @param[in]       DataSize         Size of Data buffer.\r
+  @param[in]       FreeBufSize      Size of free data buffer \r
+  @param[in]       NewData          Pointer to new EFI_SIGNATURE_LIST to be appended.\r
+  @param[in]       NewDataSize      Size of NewData buffer.\r
+  @param[out]      MergedBufSize    Size of the merged buffer\r
 \r
-  @return Size of the merged buffer.\r
+  @return EFI_BUFFER_TOO_SMALL if input Data buffer overflowed\r
 \r
 **/\r
-UINTN\r
+EFI_STATUS\r
 AppendSignatureList (\r
   IN  OUT VOID            *Data,\r
   IN  UINTN               DataSize,\r
+  IN  UINTN               FreeBufSize,\r
   IN  VOID                *NewData,\r
-  IN  UINTN               NewDataSize\r
+  IN  UINTN               NewDataSize,\r
+  OUT UINTN               *MergedBufSize\r
   );\r
 \r
 /**\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
index ebe04b50f5bd2d710e05ace2d67138e33ed18db7..f10d8f7d8358a6344033e8ddfe256a63ded017c6 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
@@ -1649,7 +1649,7 @@ UpdateVariable (
   EFI_STATUS                          Status;\r
   VARIABLE_HEADER                     *NextVariable;\r
   UINTN                               ScratchSize;\r
-  UINTN                               ScratchDataSize;\r
+  UINTN                               MaxDataSize;\r
   UINTN                               NonVolatileVarableStoreSize;\r
   UINTN                               VarNameOffset;\r
   UINTN                               VarDataOffset;\r
@@ -1664,7 +1664,6 @@ UpdateVariable (
   UINTN                               CacheOffset;\r
   UINTN                               BufSize;\r
   UINTN                               DataOffset;\r
-  UINTN                               RevBufSize;\r
 \r
   if (mVariableModuleGlobal->FvbInstance == NULL) {\r
     //\r
@@ -1713,7 +1712,7 @@ UpdateVariable (
   //\r
   NextVariable = GetEndPointer ((VARIABLE_STORE_HEADER *) ((UINTN) mVariableModuleGlobal->VariableGlobal.VolatileVariableBase));\r
   ScratchSize = MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32 (PcdMaxHardwareErrorVariableSize));\r
-  ScratchDataSize = ScratchSize - sizeof (VARIABLE_HEADER) - StrSize (VariableName) - GET_PAD_SIZE (StrSize (VariableName));\r
+\r
 \r
   if (Variable->CurrPtr != NULL) {\r
     //\r
@@ -1827,14 +1826,36 @@ UpdateVariable (
         DataOffset = sizeof (VARIABLE_HEADER) + Variable->CurrPtr->NameSize + GET_PAD_SIZE (Variable->CurrPtr->NameSize);\r
         CopyMem (mStorageArea, (UINT8*)((UINTN) Variable->CurrPtr + DataOffset), Variable->CurrPtr->DataSize);\r
 \r
+        //\r
+        // Set Max Common Variable Data Size as default MaxDataSize \r
+        //\r
+        MaxDataSize = PcdGet32 (PcdMaxVariableSize) - sizeof (VARIABLE_HEADER) - StrSize (VariableName) - GET_PAD_SIZE (StrSize (VariableName));\r
+\r
+\r
         if ((CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) &&\r
             ((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) || (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0))) ||\r
             (CompareGuid (VendorGuid, &gEfiGlobalVariableGuid) && (StrCmp (VariableName, EFI_KEY_EXCHANGE_KEY_NAME) == 0))) {\r
+\r
           //\r
           // For variables with formatted as EFI_SIGNATURE_LIST, the driver shall not perform an append of\r
           // EFI_SIGNATURE_DATA values that are already part of the existing variable value.\r
           //\r
-          BufSize = AppendSignatureList (mStorageArea, Variable->CurrPtr->DataSize, Data, DataSize);\r
+          Status = AppendSignatureList (\r
+                     mStorageArea, \r
+                     Variable->CurrPtr->DataSize, \r
+                     MaxDataSize - Variable->CurrPtr->DataSize,\r
+                     Data, \r
+                     DataSize, \r
+                     &BufSize\r
+                     );\r
+          if (Status == EFI_BUFFER_TOO_SMALL) {\r
+            //\r
+            // Signture List is too long, Failed to Append\r
+            //\r
+            Status = EFI_INVALID_PARAMETER;\r
+            goto Done;\r
+          }\r
+\r
           if (BufSize == Variable->CurrPtr->DataSize) {\r
             if ((TimeStamp == NULL) || CompareTimeStamp (TimeStamp, &Variable->CurrPtr->TimeStamp)) {\r
               //\r
@@ -1849,20 +1870,23 @@ UpdateVariable (
         } else {\r
           //\r
           // For other Variables, append the new data to the end of previous data.\r
+          // Max Harware error record variable data size is different from common variable\r
           //\r
+          if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) == EFI_VARIABLE_HARDWARE_ERROR_RECORD) {\r
+            MaxDataSize = PcdGet32 (PcdMaxHardwareErrorVariableSize) - sizeof (VARIABLE_HEADER) - StrSize (VariableName) - GET_PAD_SIZE (StrSize (VariableName));\r
+          }\r
+\r
+          if (Variable->CurrPtr->DataSize + DataSize > MaxDataSize) {\r
+            //\r
+            // Exsiting data + Appended data exceed maximum variable size limitation \r
+            //\r
+            Status = EFI_INVALID_PARAMETER;\r
+            goto Done;\r
+          }\r
           CopyMem ((UINT8*)((UINTN) mStorageArea + Variable->CurrPtr->DataSize), Data, DataSize);\r
           BufSize = Variable->CurrPtr->DataSize + DataSize;\r
         }\r
 \r
-        RevBufSize = MIN (PcdGet32 (PcdMaxVariableSize), ScratchDataSize);\r
-        if (BufSize > RevBufSize) {\r
-          //\r
-          // If variable size (previous + current) is bigger than reserved buffer in runtime,\r
-          // return EFI_OUT_OF_RESOURCES.\r
-          //\r
-          return EFI_OUT_OF_RESOURCES;\r
-        }\r
-\r
         //\r
         // Override Data and DataSize which are used for combined data area including previous and new data.\r
         //\r