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

diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
index 35862c62232c2847db15c46ae67e04e4546ded21..b9c819058b717d3f37beb18cc45d81cba74ec3b9 100644 (file)
--- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
+++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
@@ -1,6 +1,14 @@
 /** @file\r
 \r
-Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>\r
+\r
+  Caution: This module requires additional review when modified.\r
+  This driver will have external input - communicate buffer in SMM mode.\r
+  This external input must be validated carefully to avoid security issue like\r
+  buffer overflow, integer overflow.\r
+\r
+  SmmLockBoxHandler(), SmmLockBoxRestore(), SmmLockBoxUpdate(), SmmLockBoxSave()\r
+  will receive untrusted input and do basic validation.\r
 \r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions\r
@@ -60,9 +68,40 @@ IsAddressInSmram (
   return FALSE;\r
 }\r
 \r
+/**\r
+  This function check if the address refered by Buffer and Length is valid.\r
+\r
+  @param Buffer  the buffer address to be checked.\r
+  @param Length  the buffer length to be checked.\r
+\r
+  @retval TRUE  this address is valid.\r
+  @retval FALSE this address is NOT valid.\r
+**/\r
+BOOLEAN\r
+IsAddressValid (\r
+  IN UINTN                 Buffer,\r
+  IN UINTN                 Length\r
+  )\r
+{\r
+  if (Buffer > (MAX_ADDRESS - Length)) {\r
+    //\r
+    // Overflow happen\r
+    //\r
+    return FALSE;\r
+  }\r
+  if (IsAddressInSmram ((EFI_PHYSICAL_ADDRESS)Buffer, (UINT64)Length)) {\r
+    return FALSE;\r
+  }\r
+  return TRUE;\r
+}\r
+\r
 /**\r
   Dispatch function for SMM lock box save.\r
 \r
+  Caution: This function may receive untrusted input.\r
+  Restore buffer and length are external input, so this function will validate\r
+  it is in SMRAM.\r
+\r
   @param LockBoxParameterSave  parameter of lock box save \r
 **/\r
 VOID\r
@@ -81,6 +120,15 @@ SmmLockBoxSave (
     return ;\r
   }\r
 \r
+  //\r
+  // Sanity check\r
+  //\r
+  if (!IsAddressValid ((UINTN)LockBoxParameterSave->Buffer, (UINTN)LockBoxParameterSave->Length)) {\r
+    DEBUG ((EFI_D_ERROR, "SmmLockBox Save address in SMRAM!\n"));\r
+    LockBoxParameterSave->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;\r
+    return ;\r
+  }\r
+\r
   //\r
   // Save data\r
   //\r
@@ -128,6 +176,10 @@ SmmLockBoxSetAttributes (
 /**\r
   Dispatch function for SMM lock box update.\r
 \r
+  Caution: This function may receive untrusted input.\r
+  Restore buffer and length are external input, so this function will validate\r
+  it is in SMRAM.\r
+\r
   @param LockBoxParameterUpdate  parameter of lock box update \r
 **/\r
 VOID\r
@@ -146,6 +198,15 @@ SmmLockBoxUpdate (
     return ;\r
   }\r
 \r
+  //\r
+  // Sanity check\r
+  //\r
+  if (!IsAddressValid ((UINTN)LockBoxParameterUpdate->Buffer, (UINTN)LockBoxParameterUpdate->Length)) {\r
+    DEBUG ((EFI_D_ERROR, "SmmLockBox Update address in SMRAM!\n"));\r
+    LockBoxParameterUpdate->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;\r
+    return ;\r
+  }\r
+\r
   //\r
   // Update data\r
   //\r
@@ -162,6 +223,10 @@ SmmLockBoxUpdate (
 /**\r
   Dispatch function for SMM lock box restore.\r
 \r
+  Caution: This function may receive untrusted input.\r
+  Restore buffer and length are external input, so this function will validate\r
+  it is in SMRAM.\r
+\r
   @param LockBoxParameterRestore  parameter of lock box restore \r
 **/\r
 VOID\r
@@ -174,7 +239,7 @@ SmmLockBoxRestore (
   //\r
   // Sanity check\r
   //\r
-  if (IsAddressInSmram (LockBoxParameterRestore->Buffer, LockBoxParameterRestore->Length)) {\r
+  if (!IsAddressValid ((UINTN)LockBoxParameterRestore->Buffer, (UINTN)LockBoxParameterRestore->Length)) {\r
     DEBUG ((EFI_D_ERROR, "SmmLockBox Restore address in SMRAM!\n"));\r
     LockBoxParameterRestore->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;\r
     return ;\r
@@ -220,6 +285,9 @@ SmmLockBoxRestoreAllInPlace (
 /**\r
   Dispatch function for a Software SMI handler.\r
 \r
+  Caution: This function may receive untrusted input.\r
+  Communicate buffer and buffer size are external input, so this function will do basic validation.\r
+\r
   @param DispatchHandle  The unique handle assigned to this handler by SmiHandlerRegister().\r
   @param Context         Points to an optional handler context which was specified when the\r
                          handler was registered.\r
@@ -243,6 +311,18 @@ SmmLockBoxHandler (
 \r
   DEBUG ((EFI_D_ERROR, "SmmLockBox SmmLockBoxHandler Enter\n"));\r
 \r
+  //\r
+  // Sanity check\r
+  //\r
+  if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_HEADER)) {\r
+    DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size invalid!\n"));\r
+    return EFI_SUCCESS;\r
+  }\r
+  if (!IsAddressValid ((UINTN)CommBuffer, *CommBufferSize)) {\r
+    DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer in SMRAM!\n"));\r
+    return EFI_SUCCESS;\r
+  }\r
+\r
   LockBoxParameterHeader = (EFI_SMM_LOCK_BOX_PARAMETER_HEADER *)((UINTN)CommBuffer);\r
 \r
   LockBoxParameterHeader->ReturnStatus = (UINT64)-1;\r
@@ -253,21 +333,42 @@ SmmLockBoxHandler (
 \r
   switch (LockBoxParameterHeader->Command) {\r
   case EFI_SMM_LOCK_BOX_COMMAND_SAVE:\r
+    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)) {\r
+      DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for SAVE invalid!\n"));\r
+      break;\r
+    }\r
     SmmLockBoxSave ((EFI_SMM_LOCK_BOX_PARAMETER_SAVE *)(UINTN)LockBoxParameterHeader);\r
     break;\r
   case EFI_SMM_LOCK_BOX_COMMAND_UPDATE:\r
+    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)) {\r
+      DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for UPDATE invalid!\n"));\r
+      break;\r
+    }\r
     SmmLockBoxUpdate ((EFI_SMM_LOCK_BOX_PARAMETER_UPDATE *)(UINTN)LockBoxParameterHeader);\r
     break;\r
   case EFI_SMM_LOCK_BOX_COMMAND_RESTORE:\r
+    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)) {\r
+      DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for RESTORE invalid!\n"));\r
+      break;\r
+    }\r
     SmmLockBoxRestore ((EFI_SMM_LOCK_BOX_PARAMETER_RESTORE *)(UINTN)LockBoxParameterHeader);\r
     break;\r
   case EFI_SMM_LOCK_BOX_COMMAND_SET_ATTRIBUTES:\r
+    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)) {\r
+      DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for SET_ATTRIBUTES invalid!\n"));\r
+      break;\r
+    }\r
     SmmLockBoxSetAttributes ((EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES *)(UINTN)LockBoxParameterHeader);\r
     break;\r
   case EFI_SMM_LOCK_BOX_COMMAND_RESTORE_ALL_IN_PLACE:\r
+    if (*CommBufferSize < sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)) {\r
+      DEBUG ((EFI_D_ERROR, "SmmLockBox Command Buffer Size for RESTORE_ALL_IN_PLACE invalid!\n"));\r
+      break;\r
+    }\r
     SmmLockBoxRestoreAllInPlace ((EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE *)(UINTN)LockBoxParameterHeader);\r
     break;\r
   default:\r
+    DEBUG ((EFI_D_ERROR, "SmmLockBox Command invalid!\n"));\r
     break;\r
   }\r
 \r

diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
index 94ec412221f7b2550efb63c5201915aa21cbe99f..559d39f3482d3bdf27e4d05a0b1830b2dd443208 100644 (file)
--- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
+++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
@@ -1,7 +1,12 @@
 ## @file\r
 #  Component description file for LockBox SMM driver.\r
 #\r
-#  Copyright (c) 2010, Intel Corporation. All rights reserved.<BR>\r
+#  Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>\r
+#\r
+#  Caution: This module requires additional review when modified.\r
+#  This driver will have external input - communicate buffer in SMM mode.\r
+#  This external input must be validated carefully to avoid security issue like\r
+#  buffer overflow, integer overflow.\r
 #\r
 #  This program and the accompanying materials\r
 #  are licensed and made available under the terms and conditions\r