Add comments to clarify mPubKeyStore buffer MemCopy. There is no memory overflow...

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao, Zhang <chao.b.zhang@intel.com>
Reviewed-by: Yao, Jiewen <jiewen.yao@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16227 6f19259b-4bc3-4df7-8a09-765794883524

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
index 49d7648f66557f11a1071791529b1fc5be201100..566c5e29a7866cc4248f01fc56df13fed3e08816 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
@@ -7,6 +7,10 @@
   This external input must be validated carefully to avoid security issue like\r
   buffer overflow, integer overflow.\r
   Variable attribute should also be checked to avoid authentication bypass.\r
+     The whole SMM authentication variable design relies on the integrity of flash part and SMM.\r
+  which is assumed to be protected by platform.  All variable code and metadata in flash/SMM Memory\r
+  may not be modified without authorization. If platform fails to protect these resources, \r
+  the authentication service provided in this driver will be broken, and the behavior is undefined.\r
 \r
   ProcessVarWithPk(), ProcessVarWithKek() and ProcessVariable() are the function to do\r
   variable authentication.\r
@@ -251,6 +255,10 @@ AutenticatedVariableServiceInitialize (
     DataSize  = DataSizeOfVariable (Variable.CurrPtr);\r
     Data      = GetVariableDataPtr (Variable.CurrPtr);\r
     ASSERT ((DataSize != 0) && (Data != NULL));\r
+    //\r
+    // "AuthVarKeyDatabase" is an internal variable. Its DataSize is always ensured not to exceed mPubKeyStore buffer size(See definition before) \r
+    //  Therefore, there is no memory overflow in underlying CopyMem.\r
+    //\r
     CopyMem (mPubKeyStore, (UINT8 *) Data, DataSize);\r
     mPubKeyNumber = (UINT32) (DataSize / EFI_CERT_TYPE_RSA2048_SIZE);\r
   }\r
@@ -564,6 +572,10 @@ AddPubKeyInStore (
       DataSize  = DataSizeOfVariable (Variable.CurrPtr);\r
       Data      = GetVariableDataPtr (Variable.CurrPtr);\r
       ASSERT ((DataSize != 0) && (Data != NULL));\r
+      //\r
+      // "AuthVarKeyDatabase" is an internal used variable. Its DataSize is always ensured not to exceed mPubKeyStore buffer size(See definition before) \r
+      //  Therefore, there is no memory overflow in underlying CopyMem.\r
+      //\r
       CopyMem (mPubKeyStore, (UINT8 *) Data, DataSize);\r
       mPubKeyNumber = (UINT32) (DataSize / EFI_CERT_TYPE_RSA2048_SIZE);\r
 \r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
index 6ebc777075626f989348d58c8b1e20f515ff5e3b..0e57c5b5e5a46733bc68e33f2ad0997eb840fe71 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
@@ -2,6 +2,16 @@
   The internal header file includes the common header files, defines\r
   internal structure and functions used by AuthService module.\r
 \r
+  Caution: This module requires additional review when modified.\r
+  This driver will have external input - variable data. It may be input in SMM mode.\r
+  This external input must be validated carefully to avoid security issue like\r
+  buffer overflow, integer overflow.\r
+  Variable attribute should also be checked to avoid authentication bypass.\r
+     The whole SMM authentication variable design relies on the integrity of flash part and SMM.\r
+  which is assumed to be protected by platform.  All variable code and metadata in flash/SMM Memory\r
+  may not be modified without authorization. If platform fails to protect these resources, \r
+  the authentication service provided in this driver will be broken, and the behavior is undefined.\r
+\r
 Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.<BR>\r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf
index 279a9248e292e9a93ead06b49cdf7afc8162c949..1987764d803c40802fb12874d7ee3d5eb2c0fc63 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf
@@ -13,6 +13,10 @@
 #  This driver will have external input - variable data and communicate buffer in SMM mode.\r
 #  This external input must be validated carefully to avoid security issues such as \r
 #  buffer overflow or integer overflow.\r
+#    The whole SMM authentication variable design relies on the integrity of flash part and SMM.\r
+#  which is assumed to be protected by platform.  All variable code and metadata in flash/SMM Memory\r
+#  may not be modified without authorization. If platform fails to protect these resources, \r
+#  the authentication service provided in this driver will be broken, and the behavior is undefined.\r
 #\r
 # Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>\r
 # This program and the accompanying materials\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.inf b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.inf
index f0dfa89810bdef09881fb10c43c7e38b383f50aa..ca6027433918cd537b6c2fa65ee172d8249b378b 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.inf
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.inf
@@ -8,6 +8,10 @@
 #  This driver will have external input - variable data.\r
 #  This external input must be validated carefully to avoid security issues such as \r
 #  buffer overflow or integer overflow.\r
+#    The whole SMM authentication variable design relies on the integrity of flash part and SMM.\r
+#  which is assumed to be protected by platform.  All variable code and metadata in flash/SMM Memory\r
+#  may not be modified without authorization. If platform fails to protect these resources, \r
+#  the authentication service provided in this driver will be broken, and the behavior is undefined.\r
 #\r
 # Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>\r
 # This program and the accompanying materials\r