Add more security check for CommBuffer+CommBufferSize.

signed off by: jiewen.yao@intel.com
reviewed by: rui.sun@intel.com
reviewed by: michael.d.kinney@intel.com

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

2 files changed, 109 insertions, 3 deletions

diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
index 35862c6223..b9c819058b 100644
--- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
+++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
@@ -1,6 +1,14 @@
 /** @file

 

-Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved.<BR>

+Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>

+

+  Caution: This module requires additional review when modified.

+  This driver will have external input - communicate buffer in SMM mode.

+  This external input must be validated carefully to avoid security issue like

+  buffer overflow, integer overflow.

+

+  SmmLockBoxHandler(), SmmLockBoxRestore(), SmmLockBoxUpdate(), SmmLockBoxSave()

+  will receive untrusted input and do basic validation.

 

 This program and the accompanying materials

 are licensed and made available under the terms and conditions

@@ -61,8 +69,39 @@ IsAddressInSmram (
 }

 

 /**

+  This function check if the address refered by Buffer and Length is valid.

+

+  @param Buffer  the buffer address to be checked.

+  @param Length  the buffer length to be checked.

+

+  @retval TRUE  this address is valid.

+  @retval FALSE this address is NOT valid.

+**/

+BOOLEAN

+IsAddressValid (

+  IN UINTN                 Buffer,

+  IN UINTN                 Length

+  )

+{

+  if (Buffer > (MAX_ADDRESS - Length)) {

+    //

+    // Overflow happen

+    //

+    return FALSE;

+  }

+  if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {

+    return FALSE;

+  }

+  return TRUE;

+}

+

+/**

   Dispatch function for SMM lock box save.

 

+  Caution: This function may receive untrusted input.

+  Restore buffer and length are external input, so this function will validate

+  it is in SMRAM.

+

   @param LockBoxParameterSave  parameter of lock box save 

 **/

 VOID

@@ -82,6 +121,15 @@ SmmLockBoxSave (
   }

 

   //

+  // Sanity check

+  //

+  if (!IsAddressValid ((UINTN)LockBoxParameterSave->Buffer, (UINTN)LockBoxParameterSave->Length)) {

+    DEBUG ((EFI_D_ERROR, "SmmLockBox Save address in SMRAM!\n"));

+    LockBoxParameterSave->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;

+    return ;

+  }

+

+  //

   // Save data

   //

   Status = SaveLockBox (

@@ -128,6 +176,10 @@ SmmLockBoxSetAttributes (
 /**

   Dispatch function for SMM lock box update.

 

+  Caution: This function may receive untrusted input.

+  Restore buffer and length are external input, so this function will validate

+  it is in SMRAM.

+

   @param LockBoxParameterUpdate  parameter of lock box update 

 **/

 VOID

@@ -147,6 +199,15 @@ SmmLockBoxUpdate (
   }

 

   //

+  // Sanity check

+  //

+  if (!IsAddressValid ((UINTN)LockBoxParameterUpdate->Buffer, (UINTN)LockBoxParameterUpdate->Length)) {

+    DEBUG ((EFI_D_ERROR, "SmmLockBox Update address in SMRAM!\n"));

+    LockBoxParameterUpdate->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;

+    return ;

+  }

+

+  //

   // Update data

   //

   Status = UpdateLockBox (

@@ -162,6 +223,10 @@ SmmLockBoxUpdate (
 /**

   Dispatch function for SMM lock box restore.

 

+  Caution: This function may receive untrusted input.

+  Restore buffer and length are external input, so this function will validate

+  it is in SMRAM.

+

   @param LockBoxParameterRestore  parameter of lock box restore 

 **/

 VOID

@@ -174,7 +239,7 @@ SmmLockBoxRestore (
   //

   // Sanity check

   //

-  if (IsAddressInSmram (LockBoxParameterRestore->Buffer, LockBoxParameterRestore->Length)) {

+  if (!IsAddressValid ((UINTN)LockBoxParameterRestore->Buffer, (UINTN)LockBoxParameterRestore->Length)) {

     DEBUG ((EFI_D_ERROR, "SmmLockBox Restore address in SMRAM!\n"));

     LockBoxParameterRestore->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;

     return ;

@@ -220,6 +285,9 @@ SmmLockBoxRestoreAllInPlace (
 /**

   Dispatch function for a Software SMI handler.

 

+  Caution: This function may receive untrusted input.

+  Communicate buffer and buffer size are external input, so this function will do basic validation.

+

   @param DispatchHandle  The unique handle assigned to this handler by SmiHandlerRegister().

   @param Context         Points to an optional handler context which was specified when the

                          handler was registered.

@@ -243,6 +311,18 @@ SmmLockBoxHandler (
 

   DEBUG ((EFI_D_ERROR, "SmmLockBox SmmLockBoxHandler Enter\n"));

 

+  //

+  // Sanity check

+  //

+  if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_HEADER)) {

+    DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size invalid!\n"));

+    return EFI_SUCCESS;

+  }

+  if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {

+    DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer in SMRAM!\n"));

+    return EFI_SUCCESS;

+  }

+

   LockBoxParameterHeader = (EFI_SMM_LOCK_BOX_PARAMETER_HEADER *)((UINTN)CommBuffer);

 

   LockBoxParameterHeader->ReturnStatus = (UINT64)-1;

@@ -253,21 +333,42 @@ SmmLockBoxHandler (
 

   switch (LockBoxParameterHeader->Command) {

   case EFI_SMM_LOCK_BOX_COMMAND_SAVE:

+    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)) {

+      DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for SAVE invalid!\n"));

+      break;

+    }

     SmmLockBoxSave ((EFI_SMM_LOCK_BOX_PARAMETER_SAVE *)(UINTN)LockBoxParameterHeader);

     break;

   case EFI_SMM_LOCK_BOX_COMMAND_UPDATE:

+    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)) {

+      DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for UPDATE invalid!\n"));

+      break;

+    }

     SmmLockBoxUpdate ((EFI_SMM_LOCK_BOX_PARAMETER_UPDATE *)(UINTN)LockBoxParameterHeader);

     break;

   case EFI_SMM_LOCK_BOX_COMMAND_RESTORE:

+    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)) {

+      DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for RESTORE invalid!\n"));

+      break;

+    }

     SmmLockBoxRestore ((EFI_SMM_LOCK_BOX_PARAMETER_RESTORE *)(UINTN)LockBoxParameterHeader);

     break;

   case EFI_SMM_LOCK_BOX_COMMAND_SET_ATTRIBUTES:

+    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)) {

+      DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for SET_ATTRIBUTES invalid!\n"));

+      break;

+    }

     SmmLockBoxSetAttributes ((EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES *)(UINTN)LockBoxParameterHeader);

     break;

   case EFI_SMM_LOCK_BOX_COMMAND_RESTORE_ALL_IN_PLACE:

+    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)) {

+      DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for RESTORE_ALL_IN_PLACE invalid!\n"));

+      break;

+    }

     SmmLockBoxRestoreAllInPlace ((EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE *)(UINTN)LockBoxParameterHeader);

     break;

   default:

+    DEBUG ((EFI_D_ERROR, "SmmLockBox Command invalid!\n"));

     break;

   }

 

diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
index 94ec412221..559d39f348 100644
--- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
+++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
@@ -1,7 +1,12 @@
 ## @file

 #  Component description file for LockBox SMM driver.

 #

-#  Copyright (c) 2010, Intel Corporation. All rights reserved.<BR>

+#  Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>

+#

+#  Caution: This module requires additional review when modified.

+#  This driver will have external input - communicate buffer in SMM mode.

+#  This external input must be validated carefully to avoid security issue like

+#  buffer overflow, integer overflow.

 #

 #  This program and the accompanying materials

 #  are licensed and made available under the terms and conditions