[PATCH 2/2] UefiCpuPkg/Microcode: Add verification logic before parsing payload

[Chen A Chen <chen.a.chen@intel.com>]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020

The function MicrocodeDetect may receive untrusted input. Add verification
logic to check Microcode Payload Header.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/Microcode.c | 190 +++++++++++++++++++++----------
 1 file changed, 127 insertions(+), 63 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 5f9ae22794..1d5e964791 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -32,6 +32,69 @@ GetCurrentMicrocodeSignature (
   return BiosSignIdMsr.Bits.MicrocodeUpdateSignature;
 }
 
+/**
+  Verify Microcode.
+
+  @param[in]  MicrocodeHeader       Point to the Microcode payload buffer.
+
+  @retval     TRUE                  Valid payload
+  @retval     FALSE                 Invalid payload
+**/
+BOOLEAN
+VerifyMicrocodeHeader (
+  IN CPU_MICROCODE_HEADER *MicrocodeHeader
+  )
+{
+  UINTN                   TotalSize;
+  UINTN                   DataSize;
+
+  if (MicrocodeHeader == NULL) {
+    return FALSE;
+  }
+
+  ///
+  /// Verify Version
+  ///
+  if (MicrocodeHeader->HeaderVersion != 0x1) {
+    return FALSE;
+  }
+  if (MicrocodeHeader->LoaderRevision != 0x1) {
+    return FALSE;
+  }
+
+  ///
+  /// Verify TotalSize
+  ///
+  if (MicrocodeHeader->DataSize == 0) {
+    TotalSize = 2048;
+  } else {
+    TotalSize = MicrocodeHeader->TotalSize;
+  }
+  if (TotalSize <= sizeof (CPU_MICROCODE_HEADER)) {
+    return FALSE;
+  }
+  if ((TotalSize & (SIZE_1KB - 1)) != 0) {
+    return FALSE;
+  }
+
+  ///
+  /// Verify DataSize
+  ///
+  if (MicrocodeHeader->DataSize == 0) {
+    DataSize = 2048 - sizeof (CPU_MICROCODE_HEADER);
+  } else {
+    DataSize = MicrocodeHeader->DataSize;
+  }
+  if (DataSize > TotalSize - sizeof (CPU_MICROCODE_HEADER)) {
+    return FALSE;
+  }
+  if ((DataSize & 0x3) != 0) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
 /**
   Detect whether specified processor can find matching microcode patch and load it.
 
@@ -166,6 +229,18 @@ MicrocodeDetect (
     //
     CorrectMicrocode = FALSE;
 
+    if (!VerifyMicrocodeHeader (MicrocodeEntryPoint)) {
+      //
+      // It is the padding data between the microcode patches for microcode patches alignment.
+      // Because the microcode patch is the multiple of 1-KByte, the padding data should not
+      // exist if the microcode patch alignment value is not larger than 1-KByte. So, the microcode
+      // alignment value should be larger than 1-KByte. We could skip SIZE_1KB padding data to
+      // find the next possible microcode patch header.
+      //
+      MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + SIZE_1KB);
+      continue;
+    }
+
     //
     // Save an in-complete CheckSum32 from CheckSum Part1 for common parts.
     //
@@ -184,90 +259,79 @@ MicrocodeDetect (
     InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
     InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
 
-    if (MicrocodeEntryPoint->HeaderVersion == 0x1) {
+    //
+    // It is the microcode header. It is not the padding data between microcode patches
+    // because the padding data should not include 0x00000001 and it should be the repeated
+    // byte format (like 0xXYXYXYXY....).
+    //
+    if (MicrocodeEntryPoint->ProcessorSignature.Uint32 == Eax.Uint32 &&
+        MicrocodeEntryPoint->UpdateRevision > LatestRevision &&
+        (MicrocodeEntryPoint->ProcessorFlags & (1 << PlatformId))
+        ) {
       //
-      // It is the microcode header. It is not the padding data between microcode patches
-      // because the padding data should not include 0x00000001 and it should be the repeated
-      // byte format (like 0xXYXYXYXY....).
+      // Calculate CheckSum Part1.
       //
-      if (MicrocodeEntryPoint->ProcessorSignature.Uint32 == Eax.Uint32 &&
-          MicrocodeEntryPoint->UpdateRevision > LatestRevision &&
-          (MicrocodeEntryPoint->ProcessorFlags & (1 << PlatformId))
-          ) {
+      CheckSum32 = InCompleteCheckSum32;
+      CheckSum32 += MicrocodeEntryPoint->ProcessorSignature.Uint32;
+      CheckSum32 += MicrocodeEntryPoint->ProcessorFlags;
+      CheckSum32 += MicrocodeEntryPoint->Checksum;
+      if (CheckSum32 == 0) {
+        CorrectMicrocode = TRUE;
+        ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
+      }
+    } else if ((MicrocodeEntryPoint->DataSize != 0) &&
+               (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {
+      ExtendedTableLength = MicrocodeEntryPoint->TotalSize - (MicrocodeEntryPoint->DataSize +
+                              sizeof (CPU_MICROCODE_HEADER));
+      if (ExtendedTableLength != 0) {
         //
-        // Calculate CheckSum Part1.
+        // Extended Table exist, check if the CPU in support list
         //
-        CheckSum32 = InCompleteCheckSum32;
-        CheckSum32 += MicrocodeEntryPoint->ProcessorSignature.Uint32;
-        CheckSum32 += MicrocodeEntryPoint->ProcessorFlags;
-        CheckSum32 += MicrocodeEntryPoint->Checksum;
-        if (CheckSum32 == 0) {
-          CorrectMicrocode = TRUE;
-          ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
-        }
-      } else if ((MicrocodeEntryPoint->DataSize != 0) &&
-                 (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {
-        ExtendedTableLength = MicrocodeEntryPoint->TotalSize - (MicrocodeEntryPoint->DataSize +
-                                sizeof (CPU_MICROCODE_HEADER));
-        if (ExtendedTableLength != 0) {
-          //
-          // Extended Table exist, check if the CPU in support list
-          //
-          ExtendedTableHeader = (CPU_MICROCODE_EXTENDED_TABLE_HEADER *) ((UINT8 *) (MicrocodeEntryPoint)
-                                  + MicrocodeEntryPoint->DataSize + sizeof (CPU_MICROCODE_HEADER));
+        ExtendedTableHeader = (CPU_MICROCODE_EXTENDED_TABLE_HEADER *) ((UINT8 *) (MicrocodeEntryPoint)
+                                + MicrocodeEntryPoint->DataSize + sizeof (CPU_MICROCODE_HEADER));
+        //
+        // Calculate Extended Checksum
+        //
+        if ((ExtendedTableLength % 4) == 0) {
           //
-          // Calculate Extended Checksum
+          // Calculate CheckSum Part2.
           //
-          if ((ExtendedTableLength % 4) == 0) {
+          CheckSum32 = CalculateSum32 ((UINT32 *) ExtendedTableHeader, ExtendedTableLength);
+          if (CheckSum32 == 0) {
             //
-            // Calculate CheckSum Part2.
+            // Checksum correct
             //
-            CheckSum32 = CalculateSum32 ((UINT32 *) ExtendedTableHeader, ExtendedTableLength);
-            if (CheckSum32 == 0) {
+            ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
+            ExtendedTable      = (CPU_MICROCODE_EXTENDED_TABLE *) (ExtendedTableHeader + 1);
+            for (Index = 0; Index < ExtendedTableCount; Index ++) {
               //
-              // Checksum correct
+              // Calculate CheckSum Part3.
               //
-              ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
-              ExtendedTable      = (CPU_MICROCODE_EXTENDED_TABLE *) (ExtendedTableHeader + 1);
-              for (Index = 0; Index < ExtendedTableCount; Index ++) {
+              CheckSum32 = InCompleteCheckSum32;
+              CheckSum32 += ExtendedTable->ProcessorSignature.Uint32;
+              CheckSum32 += ExtendedTable->ProcessorFlag;
+              CheckSum32 += ExtendedTable->Checksum;
+              if (CheckSum32 == 0) {
                 //
-                // Calculate CheckSum Part3.
+                // Verify Header
                 //
-                CheckSum32 = InCompleteCheckSum32;
-                CheckSum32 += ExtendedTable->ProcessorSignature.Uint32;
-                CheckSum32 += ExtendedTable->ProcessorFlag;
-                CheckSum32 += ExtendedTable->Checksum;
-                if (CheckSum32 == 0) {
+                if ((ExtendedTable->ProcessorSignature.Uint32 == Eax.Uint32) &&
+                    (ExtendedTable->ProcessorFlag & (1 << PlatformId)) ) {
                   //
-                  // Verify Header
+                  // Find one
                   //
-                  if ((ExtendedTable->ProcessorSignature.Uint32 == Eax.Uint32) &&
-                      (ExtendedTable->ProcessorFlag & (1 << PlatformId)) ) {
-                    //
-                    // Find one
-                    //
-                    CorrectMicrocode = TRUE;
-                    ProcessorFlags = ExtendedTable->ProcessorFlag;
-                    break;
-                  }
+                  CorrectMicrocode = TRUE;
+                  ProcessorFlags = ExtendedTable->ProcessorFlag;
+                  break;
                 }
-                ExtendedTable ++;
               }
+              ExtendedTable ++;
             }
           }
         }
       }
-    } else {
-      //
-      // It is the padding data between the microcode patches for microcode patches alignment.
-      // Because the microcode patch is the multiple of 1-KByte, the padding data should not
-      // exist if the microcode patch alignment value is not larger than 1-KByte. So, the microcode
-      // alignment value should be larger than 1-KByte. We could skip SIZE_1KB padding data to
-      // find the next possible microcode patch header.
-      //
-      MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + SIZE_1KB);
-      continue;
     }
+
     //
     // Get the next patch.
     //
-- 
2.16.2.windows.1