From 32177f69c40f81a8cdb2033c422f3c76f57945e5 Mon Sep 17 00:00:00 2001
From: jyao1 <jyao1@6f19259b-4bc3-4df7-8a09-765794883524>
Date: Mon, 23 Jul 2012 00:59:26 +0000
Subject: [PATCH] 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
---
.../Universal/LockBox/SmmLockBox/SmmLockBox.c | 105 +++++++++++++++++-
.../LockBox/SmmLockBox/SmmLockBox.inf | 7 +-
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
@@ -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;
}
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