Revision: 13545
http://edk2.svn.sourceforge.net/edk2/?rev=13545&view=rev
Author: jyao1
Date: 2012-07-23 00:59:26 +0000 (Mon, 23 Jul 2012)
Log Message:
-----------
Add more security check for CommBuffer+CommBufferSize.
signed off by: [email protected]
reviewed by: [email protected]
reviewed by: [email protected]
Modified Paths:
--------------
trunk/edk2/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
trunk/edk2/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
Modified: trunk/edk2/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
===================================================================
--- trunk/edk2/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
2012-07-20 03:36:21 UTC (rev 13544)
+++ trunk/edk2/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
2012-07-23 00:59:26 UTC (rev 13545)
@@ -1,7 +1,15 @@
/** @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
of the BSD License which accompanies this distribution. The
@@ -61,8 +69,39 @@
}
/**
+ 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 @@
}
//
+ // 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 @@
/**
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 @@
}
//
+ // 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 @@
/**
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 @@
//
// 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 @@
/**
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 @@
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 @@
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;
}
Modified: trunk/edk2/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
===================================================================
--- trunk/edk2/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
2012-07-20 03:36:21 UTC (rev 13544)
+++ trunk/edk2/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
2012-07-23 00:59:26 UTC (rev 13545)
@@ -1,8 +1,13 @@
## @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
# of the BSD License which accompanies this distribution. The
This was sent by the SourceForge.net collaborative development platform, the
world's largest Open Source development site.
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
edk2-commits mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-commits
