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
   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
 \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
     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
     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
       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
       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
   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
 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
 #  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
 #\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
 #  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
 #\r
 # Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>\r
 # This program and the accompanying materials\r