1. Enhance AuthVar driver to avoid process corrupted certificate input.

Signed-off-by: hhuan13
Reviewed-by: ftian

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

3 files changed, 49 insertions, 19 deletions

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
index cf94182612..fc23bb5212 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
@@ -1046,19 +1046,37 @@ VerifyTimeBasedPayload (
     //

     return EFI_SECURITY_VIOLATION;

   }

-

+ 

   //

   // Find out Pkcs7 SignedData which follows the EFI_VARIABLE_AUTHENTICATION_2 descriptor.

   // AuthInfo.Hdr.dwLength is the length of the entire certificate, including the length of the header.

   //

-  SigData = (UINT8*) ((UINTN)Data + (UINTN)(((EFI_VARIABLE_AUTHENTICATION_2 *) 0)->AuthInfo.CertData));

-  SigDataSize = CertData->AuthInfo.Hdr.dwLength - (UINT32)(UINTN)(((WIN_CERTIFICATE_UEFI_GUID *) 0)->CertData);

+  SigData = (UINT8*) ((UINTN)Data + OFFSET_OF (EFI_VARIABLE_AUTHENTICATION_2, AuthInfo) + OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData));

+

+  //

+  // Sanity check to avoid corrupted certificate input.

+  //

+  if (CertData->AuthInfo.Hdr.dwLength < (UINT32)(OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData))) {

+    return EFI_SECURITY_VIOLATION;

+  }

+

+

+

+  SigDataSize = CertData->AuthInfo.Hdr.dwLength - (UINT32)(OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData));

   

   //

   // Find out the new data payload which follows Pkcs7 SignedData directly.

   //

   PayLoadPtr = (UINT8*) ((UINTN) SigData + (UINTN) SigDataSize);

-  PayLoadSize = DataSize - (UINTN)(((EFI_VARIABLE_AUTHENTICATION_2 *) 0)->AuthInfo.CertData) - (UINTN) SigDataSize;

+

+  //

+  // Sanity check to avoid corrupted certificate input.

+  //

+  if (DataSize < (OFFSET_OF (EFI_VARIABLE_AUTHENTICATION_2, AuthInfo) + OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData)+ (UINTN) SigDataSize)) {

+    return EFI_SECURITY_VIOLATION;

+  }

+  

+  PayLoadSize = DataSize - OFFSET_OF (EFI_VARIABLE_AUTHENTICATION_2, AuthInfo) - OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData) - (UINTN) SigDataSize;

 

 

   //

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
index 136bafefec..df8b30a63e 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
@@ -199,7 +199,9 @@ UpdateVariableStore (
   // Check if the Data is Volatile.

   //

   if (!Volatile) {

-    ASSERT (Fvb != NULL);

+    if (Fvb == NULL) {

+      return EFI_INVALID_PARAMETER;

+    }

     Status = Fvb->GetPhysicalAddress(Fvb, &FvVolHdr);

     ASSERT_EFI_ERROR (Status);

 

@@ -1048,7 +1050,9 @@ VariableGetBestLanguage (
   CONST CHAR8  *Supported;

   CHAR8        *Buffer;

 

-  ASSERT (SupportedLanguages != NULL);

+  if (SupportedLanguages == NULL) {

+    return NULL;

+  }

 

   VA_START (Args, Iso639Language);

   while ((Language = VA_ARG (Args, CHAR8 *)) != NULL) {

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
index 52d9aa041a..197735e399 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
@@ -241,17 +241,18 @@ GetFvbCountAndBuffer (
 /**

   Get the variable statistics information from the information buffer pointed by gVariableInfo.

 

-  @param[in, out]  InfoEntry   A pointer to the buffer of variable information entry.

-                               On input, point to the variable information returned last time. if 

-                               InfoEntry->VendorGuid is zero, return the first information.

-                               On output, point to the next variable information.

-  @param[in, out]  InfoSize    On input, the size of the variable information buffer.

-                               On output, the returned variable information size.

-

-  @retval EFI_SUCCESS          The variable information is found and returned successfully.

-  @retval EFI_UNSUPPORTED      No variable inoformation exists in variable driver. The 

-                               PcdVariableCollectStatistics should be set TRUE to support it.

-  @retval EFI_BUFFER_TOO_SMALL The buffer is too small to hold the next variable information.

+  @param[in, out]  InfoEntry    A pointer to the buffer of variable information entry.

+                                On input, point to the variable information returned last time. if 

+                                InfoEntry->VendorGuid is zero, return the first information.

+                                On output, point to the next variable information.

+  @param[in, out]  InfoSize     On input, the size of the variable information buffer.

+                                On output, the returned variable information size.

+

+  @retval EFI_SUCCESS           The variable information is found and returned successfully.

+  @retval EFI_UNSUPPORTED       No variable inoformation exists in variable driver. The 

+                                PcdVariableCollectStatistics should be set TRUE to support it.

+  @retval EFI_BUFFER_TOO_SMALL  The buffer is too small to hold the next variable information.

+  @retval EFI_INVALID_PARAMETER Input parameter is invalid.

 

 **/

 EFI_STATUS

@@ -265,7 +266,10 @@ SmmVariableGetStatistics (
   UINTN                                                StatisticsInfoSize;

   CHAR16                                               *InfoName;

  

-  ASSERT (InfoEntry != NULL);

+  if (InfoEntry == NULL) {

+    return EFI_INVALID_PARAMETER;

+  }

+  

   VariableInfo = gVariableInfo; 

   if (VariableInfo == NULL) {

     return EFI_UNSUPPORTED;

@@ -348,6 +352,8 @@ SmmVariableGetStatistics (
   @retval EFI_WARN_INTERRUPT_SOURCE_PENDING   The interrupt is still pending and other handlers should still 

                                               be called.

   @retval EFI_INTERRUPT_PENDING               The interrupt could not be quiesced.

+  @retval EFI_INVALID_PARAMETER               Input parameter is invalid.

+

 **/

 EFI_STATUS

 EFIAPI

@@ -366,7 +372,9 @@ SmmVariableHandler (
   VARIABLE_INFO_ENTRY                              *VariableInfo;

   UINTN                                            InfoSize;

 

-  ASSERT (CommBuffer != NULL);

+  if (CommBuffer == NULL) {

+    return EFI_INVALID_PARAMETER;

+  }

 

   SmmVariableFunctionHeader = (SMM_VARIABLE_COMMUNICATE_HEADER *)CommBuffer;

   switch (SmmVariableFunctionHeader->Function) {