File ovmf-bsc1196741-MdeModulePkg-PiSmmCore-SmmEntry... of Package ovmf

Overview Repositories Revisions Requests Users Attributes Meta

File ovmf-bsc1196741-MdeModulePkg-PiSmmCore-SmmEntryPoint-underflow-CVE-2.patch of Package ovmf

From cab1f02565d3b29081dd21afb074f35fdb4e1fd6 Mon Sep 17 00:00:00 2001
From: Miki Demeter <miki.demeter@intel.com>
Date: Thu, 27 Oct 2022 16:20:54 -0700
Subject: [PATCH] =?UTF-8?q?MdeModulePkg/PiSmmCore:=20SmmEntryPoint=20under?=
 =?UTF-8?q?flow=C2=A0(CVE-2021-38578)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

Added use of SafeIntLib to validate values are not causing overflows or
underflows in user controlled values when calculating buffer sizes.

Signed-off-by: Miki Demeter <miki.demeter@intel.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
---
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c   | 41 ++++++++++++++++++-----
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h   |  1 +
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf |  1 +
 MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c    | 31 +++++++++++++----
 MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf  |  1 +
 5 files changed, 60 insertions(+), 15 deletions(-)

Index: ovmf-2017+git1510945757.b2662641d5/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
===================================================================
--- ovmf-2017+git1510945757.b2662641d5.orig/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
+++ ovmf-2017+git1510945757.b2662641d5/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
@@ -446,6 +446,7 @@ SmmEndOfS3ResumeHandler (
   @param[in] Size2  Size of Buff2
 
   @retval TRUE      Buffers overlap in memory.
+  @retval TRUE      Math error.     Prevents potential math over and underflows.
   @retval FALSE     Buffer doesn't overlap.
 
 **/
@@ -457,11 +458,24 @@ InternalIsBufferOverlapped (
   IN UINTN      Size2
   )
 {
+  UINTN    End1;
+  UINTN    End2;
+  BOOLEAN  IsOverUnderflow1;
+  BOOLEAN  IsOverUnderflow2;
+
+  // Check for over or underflow
+  IsOverUnderflow1 = EFI_ERROR (SafeUintnAdd ((UINTN)Buff1, Size1, &End1));
+  IsOverUnderflow2 = EFI_ERROR (SafeUintnAdd ((UINTN)Buff2, Size2, &End2));
+
+  if (IsOverUnderflow1 || IsOverUnderflow2) {
+    return TRUE;
+  }
+
   //
   // If buff1's end is less than the start of buff2, then it's ok.
   // Also, if buff1's start is beyond buff2's end, then it's ok.
   //
-  if (((Buff1 + Size1) <= Buff2) || (Buff1 >= (Buff2 + Size2))) {
+  if ((End1 <= (UINTN)Buff2) || ((UINTN)Buff1 >= End2)) {
     return FALSE;
   }
 
@@ -487,6 +501,7 @@ SmmEntryPoint (
   EFI_SMM_COMMUNICATE_HEADER  *CommunicateHeader;
   BOOLEAN                     InLegacyBoot;
   BOOLEAN                     IsOverlapped;
+  BOOLEAN                     IsOverUnderflow;
   VOID                        *CommunicationBuffer;
   UINTN                       BufferSize;
 
@@ -537,17 +552,25 @@ SmmEntryPoint (
                        (UINT8 *) gSmmCorePrivate,
                        sizeof (*gSmmCorePrivate)
                        );
-      if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferSize) || IsOverlapped) {
+      //
+      // Check for over or underflows
+      //
+      IsOverUnderflow = EFI_ERROR (SafeUintnSub (BufferSize, OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data), &BufferSize));
+
+      if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferSize) ||
+          IsOverlapped || IsOverUnderflow)
+      {
         //
         // If CommunicationBuffer is not in valid address scope,
         // or there is overlap between gSmmCorePrivate and CommunicationBuffer,
+        // or there is over or underflow,
         // return EFI_INVALID_PARAMETER
         //
         gSmmCorePrivate->CommunicationBuffer = NULL;
         gSmmCorePrivate->ReturnStatus = EFI_INVALID_PARAMETER;
       } else {
         CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *)CommunicationBuffer;
-        BufferSize -= OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data);
+        // BufferSize was updated by the SafeUintnSub() call above.
         Status = SmiManage (
                    &CommunicateHeader->HeaderGuid, 
                    NULL, 
Index: ovmf-2017+git1510945757.b2662641d5/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
===================================================================
--- ovmf-2017+git1510945757.b2662641d5.orig/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
+++ ovmf-2017+git1510945757.b2662641d5/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
@@ -41,6 +41,7 @@
 #include <Library/ReportStatusCodeLib.h>
 
 #include "PiSmmCorePrivateData.h"
+#include <Library/SafeIntLib.h>
 
 //
 // Function prototypes from produced protocols
@@ -1253,6 +1254,7 @@ SmmSplitSmramEntry (
   @param[in] ReservedRangeToCompare     Pointer to EFI_SMM_RESERVED_SMRAM_REGION to compare.
 
   @retval TRUE  There is overlap.
+  @retval TRUE  Math error.
   @retval FALSE There is no overlap.
 
 **/
Index: ovmf-2017+git1510945757.b2662641d5/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
===================================================================
--- ovmf-2017+git1510945757.b2662641d5.orig/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ ovmf-2017+git1510945757.b2662641d5/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -59,6 +59,7 @@
 #include <Library/TimerLib.h>
 #include <Library/HobLib.h>
 #include <Library/SmmMemLib.h>
+#include <Library/SafeIntLib.h>
 
 #include "PiSmmCorePrivateData.h"
 #include "HeapGuard.h"
Index: ovmf-2017+git1510945757.b2662641d5/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
===================================================================
--- ovmf-2017+git1510945757.b2662641d5.orig/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
+++ ovmf-2017+git1510945757.b2662641d5/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
@@ -65,6 +65,7 @@
   TimerLib
   HobLib
   SmmMemLib
+  SafeIntLib
   DxeServicesLib
 
 [Protocols]

Places

File ovmf-bsc1196741-MdeModulePkg-PiSmmCore-SmmEntryPoint-underflow-CVE-2.patch of Package ovmf

Places