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

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
@@ -60,9 +68,40 @@ IsAddressInSmram (
   return FALSE;
 }
 
+/**
+  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
@@ -81,6 +120,15 @@ SmmLockBoxSave (
     return ;
   }
 
+  //
+  // 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
   //
@@ -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
@@ -146,6 +198,15 @@ SmmLockBoxUpdate (
     return ;
   }
 
+  //
+  // 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
   //
@@ -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;
   }
 

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