Add comment for modules which have external input.

signed-off-by: jiewen.yao@intel.com
reviewed-by: guo.dong@intel.com
reviewed-by: ting.ye@intel.com
reviewed-by: liming.gao@intel.com
reviewed-by: elvin.li@intel.com



git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@13446 6f19259b-4bc3-4df7-8a09-765794883524

9 files changed, 181 insertions, 3 deletions

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
index a3b620f02a..d6df32affc 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
@@ -2,6 +2,19 @@
   Implement authentication services for the authenticated variable

   service in UEFI2.2.

 

+  Caution: This module requires additional review when modified.

+  This driver will have external input - variable data. It may be input in SMM mode.

+  This external input must be validated carefully to avoid security issue like

+  buffer overflow, integer overflow.

+  Variable attribute should also be checked to avoid authentication bypass.

+

+  ProcessVarWithPk(), ProcessVarWithKek() and ProcessVariable() are the function to do

+  variable authentication.

+

+  VerifyTimeBasedPayload() and VerifyCounterBasedPayload() are sub function to do verification.

+  They will do basic validation for authentication data structure, then call crypto library

+  to verify the signature.

+

 Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.<BR>

 This program and the accompanying materials

 are licensed and made available under the terms and conditions of the BSD License

@@ -502,6 +515,12 @@ AddPubKeyInStore (
   Verify data payload with AuthInfo in EFI_CERT_TYPE_RSA2048_SHA256_GUID type.

   Follow the steps in UEFI2.2.

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize and data are external input.

+  This function will do basic validation, before parse the data.

+  This function will parse the authentication carefully to avoid security issues, like

+  buffer overflow, integer overflow.

+

   @param[in]      Data                    Pointer to data with AuthInfo.

   @param[in]      DataSize                Size of Data.

   @param[in]      PubKey                  Public key used for verification.

@@ -852,6 +871,13 @@ CheckSignatureListFormat(
 /**

   Process variable with platform key for verification.

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize and data are external input.

+  This function will do basic validation, before parse the data.

+  This function will parse the authentication carefully to avoid security issues, like

+  buffer overflow, integer overflow.

+  This function will check attribute carefully to avoid authentication bypass.

+

   @param[in]  VariableName                Name of Variable to be found.

   @param[in]  VendorGuid                  Variable vendor GUID.

   @param[in]  Data                        Data pointer.

@@ -961,6 +987,13 @@ ProcessVarWithPk (
 /**

   Process variable with key exchange key for verification.

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize and data are external input.

+  This function will do basic validation, before parse the data.

+  This function will parse the authentication carefully to avoid security issues, like

+  buffer overflow, integer overflow.

+  This function will check attribute carefully to avoid authentication bypass.

+

   @param[in]  VariableName                Name of Variable to be found.

   @param[in]  VendorGuid                  Variable vendor GUID.

   @param[in]  Data                        Data pointer.

@@ -1039,6 +1072,13 @@ ProcessVarWithKek (
 /**

   Process variable with EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS/EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS set

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize and data are external input.

+  This function will do basic validation, before parse the data.

+  This function will parse the authentication carefully to avoid security issues, like

+  buffer overflow, integer overflow.

+  This function will check attribute carefully to avoid authentication bypass.

+

   @param[in]  VariableName                Name of Variable to be found.

   @param[in]  VendorGuid                  Variable vendor GUID.

 

@@ -1806,6 +1846,12 @@ InsertCertsToDb (
 /**

   Process variable with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS set

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize and data are external input.

+  This function will do basic validation, before parse the data.

+  This function will parse the authentication carefully to avoid security issues, like

+  buffer overflow, integer overflow.

+

   @param[in]  VariableName                Name of Variable to be found.

   @param[in]  VendorGuid                  Variable vendor GUID.

   @param[in]  Data                        Data pointer.

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
index 4419c91744..7eb2a9d23c 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
@@ -76,6 +76,13 @@ typedef struct {
 /**

   Process variable with EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS/EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS set.

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize and data are external input.

+  This function will do basic validation, before parse the data.

+  This function will parse the authentication carefully to avoid security issues, like

+  buffer overflow, integer overflow.

+  This function will check attribute carefully to avoid authentication bypass.

+

   @param[in]  VariableName                Name of Variable to be found.

   @param[in]  VendorGuid                  Variable vendor GUID.

 

@@ -162,6 +169,13 @@ CheckSignatureListFormat(
 /**

   Process variable with platform key for verification.

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize and data are external input.

+  This function will do basic validation, before parse the data.

+  This function will parse the authentication carefully to avoid security issues, like

+  buffer overflow, integer overflow.

+  This function will check attribute carefully to avoid authentication bypass.

+

   @param[in]  VariableName                Name of Variable to be found.

   @param[in]  VendorGuid                  Variable vendor GUID.

   @param[in]  Data                        Data pointer.

@@ -191,6 +205,13 @@ ProcessVarWithPk (
 /**

   Process variable with key exchange key for verification.

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize and data are external input.

+  This function will do basic validation, before parse the data.

+  This function will parse the authentication carefully to avoid security issues, like

+  buffer overflow, integer overflow.

+  This function will check attribute carefully to avoid authentication bypass.

+

   @param[in]  VariableName                Name of Variable to be found.

   @param[in]  VendorGuid                  Variable vendor GUID.

   @param[in]  Data                        Data pointer.

@@ -257,6 +278,12 @@ CompareTimeStamp (
 /**

   Process variable with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS set

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize and data are external input.

+  This function will do basic validation, before parse the data.

+  This function will parse the authentication carefully to avoid security issues, like

+  buffer overflow, integer overflow.

+

   @param[in]  VariableName                Name of Variable to be found.

   @param[in]  VendorGuid                  Variable vendor GUID.

   @param[in]  Data                        Data pointer.

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
index 49358de013..fe7d5b9352 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
@@ -2,6 +2,20 @@
   The common variable operation routines shared by DXE_RUNTIME variable

   module and DXE_SMM variable module.

 

+  Caution: This module requires additional review when modified.

+  This driver will have external input - variable data. They may be input in SMM mode.

+  This external input must be validated carefully to avoid security issue like

+  buffer overflow, integer overflow.

+

+  VariableServiceGetNextVariableName () and VariableServiceQueryVariableInfo() are external API.

+  They need check input parameter.

+

+  VariableServiceGetVariable() and VariableServiceSetVariable() are external API

+  to receive datasize and data buffer. The size should be checked carefully.

+

+  VariableServiceSetVariable() should also check authenticate data to avoid buffer overflow,

+  integer overflow. It should also check attribute to avoid authentication bypass.

+

 Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.<BR>

 This program and the accompanying materials

 are licensed and made available under the terms and conditions of the BSD License

@@ -1945,6 +1959,10 @@ IsHwErrRecVariable (
 

   This code finds variable in storage blocks (Volatile or Non-Volatile).

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize is external input.

+  This function will do basic validation, before parse the data.

+

   @param VariableName               Name of Variable to be found.

   @param VendorGuid                 Variable vendor GUID.

   @param Attributes                 Attribute value of the variable found.

@@ -2022,6 +2040,9 @@ Done:
 

   This code Finds the Next available variable.

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode. This function will do basic validation, before parse the data.

+

   @param VariableNameSize           Size of the variable name.

   @param VariableName               Pointer to variable name.

   @param VendorGuid                 Variable Vendor Guid.

@@ -2167,6 +2188,13 @@ Done:
 

   This code sets variable in storage blocks (Volatile or Non-Volatile).

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize and data are external input.

+  This function will do basic validation, before parse the data.

+  This function will parse the authentication carefully to avoid security issues, like

+  buffer overflow, integer overflow.

+  This function will check attribute carefully to avoid authentication bypass.

+

   @param VariableName                     Name of Variable to be found.

   @param VendorGuid                       Variable vendor GUID.

   @param Attributes                       Attribute value of the variable found

@@ -2336,6 +2364,9 @@ VariableServiceSetVariable (
 

   This code returns information about the EFI variables.

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode. This function will do basic validation, before parse the data.

+

   @param Attributes                     Attributes bitmask to specify the type of variables

                                         on which to return information.

   @param MaximumVariableStorageSize     Pointer to the maximum size of the storage space available

@@ -2497,6 +2528,9 @@ VariableServiceQueryVariableInfo (
 /**

   This function reclaims variable storage if free size is below the threshold.

 

+  Caution: This function may be invoked at SMM runtime.

+  Care must be taken to make sure not security issue at runtime.

+

 **/

 VOID

 ReclaimForOS(

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.h b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.h
index bfb2f4e8f7..14a0744a57 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.h
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.h
@@ -402,6 +402,10 @@ GetFvbInfoByAddress (
 

   This code finds variable in storage blocks (Volatile or Non-Volatile).

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize and data are external input.

+  This function will do basic validation, before parse the data.

+

   @param VariableName               Name of Variable to be found.

   @param VendorGuid                 Variable vendor GUID.

   @param Attributes                 Attribute value of the variable found.

@@ -429,6 +433,9 @@ VariableServiceGetVariable (
 

   This code Finds the Next available variable.

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode. This function will do basic validation, before parse the data.

+

   @param VariableNameSize           Size of the variable name.

   @param VariableName               Pointer to variable name.

   @param VendorGuid                 Variable Vendor Guid.

@@ -451,6 +458,13 @@ VariableServiceGetNextVariableName (
 

   This code sets variable in storage blocks (Volatile or Non-Volatile).

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode, and datasize and data are external input.

+  This function will do basic validation, before parse the data.

+  This function will parse the authentication carefully to avoid security issues, like

+  buffer overflow, integer overflow.

+  This function will check attribute carefully to avoid authentication bypass.

+

   @param VariableName                     Name of Variable to be found.

   @param VendorGuid                       Variable vendor GUID.

   @param Attributes                       Attribute value of the variable found

@@ -479,6 +493,9 @@ VariableServiceSetVariable (
 

   This code returns information about the EFI variables.

 

+  Caution: This function may receive untrusted input.

+  This function may be invoked in SMM mode. This function will do basic validation, before parse the data.

+

   @param Attributes                     Attributes bitmask to specify the type of variables

                                         on which to return information.

   @param MaximumVariableStorageSize     Pointer to the maximum size of the storage space available

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableRuntimeDxe.inf b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableRuntimeDxe.inf
index 6765efbf2b..ab676f456a 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableRuntimeDxe.inf
@@ -1,6 +1,11 @@
 ## @file

 #  Component description file for Authenticated Variable module.

 #

+#  Caution: This module requires additional review when modified.

+#  This driver will have external input - variable data.

+#  This external input must be validated carefully to avoid security issue like

+#  buffer overflow, integer overflow.

+#

 # Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.<BR>

 # This program and the accompanying materials

 # are licensed and made available under the terms and conditions of the BSD License

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
index 197735e399..944394bef1 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.c
@@ -3,7 +3,18 @@
   implements an SMI handler to communicate with the DXE runtime driver 

   to provide variable services.

 

-Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved.<BR>

+  Caution: This module requires additional review when modified.

+  This driver will have external input - variable data and communicate buffer in SMM mode.

+  This external input must be validated carefully to avoid security issue like

+  buffer overflow, integer overflow.

+

+  SmmVariableHandler() will receive untrusted input and do basic validation.

+

+  Each sub function VariableServiceGetVariable(), VariableServiceGetNextVariableName(), 

+  VariableServiceSetVariable(), VariableServiceQueryVariableInfo(), ReclaimForOS(), 

+  SmmVariableGetStatistics() should also do validation based on its own knowledge.

+

+Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>

 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 full text of the license may be found at 

@@ -241,6 +252,9 @@ GetFvbCountAndBuffer (
 /**

   Get the variable statistics information from the information buffer pointed by gVariableInfo.

 

+  Caution: This function may be invoked at SMM runtime.

+  InfoEntry and InfoSize are external input. Care must be taken to make sure not security issue at runtime.

+

   @param[in, out]  InfoEntry    A pointer to the buffer of variable information entry.

                                 On input, point to the variable information returned last time. if 

                                 InfoEntry->VendorGuid is zero, return the first information.

@@ -338,6 +352,12 @@ SmmVariableGetStatistics (
 

   This SMI handler provides services for the variable wrapper driver.

 

+  Caution: This function may receive untrusted input.

+  This variable data and communicate buffer are external input, so this function will do basic validation.

+  Each sub function VariableServiceGetVariable(), VariableServiceGetNextVariableName(), 

+  VariableServiceSetVariable(), VariableServiceQueryVariableInfo(), ReclaimForOS(), 

+  SmmVariableGetStatistics() should also do validation based on its own knowledge.

+

   @param[in]     DispatchHandle  The unique handle assigned to this handler by SmiHandlerRegister().

   @param[in]     RegisterContext Points to an optional handler context which was specified when the

                                  handler was registered.

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf
index cb9787df9c..a6343dbbbd 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf
@@ -9,6 +9,11 @@
 #  SMM Runtime DXE module would install variable arch protocol and variable

 #  write arch protocol based on SMM variable module.

 #

+#  Caution: This module requires additional review when modified.

+#  This driver will have external input - variable data and communicate buffer in SMM mode.

+#  This external input must be validated carefully to avoid security issue like

+#  buffer overflow, integer overflow.

+#

 # Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>

 # This program and the accompanying materials

 # are licensed and made available under the terms and conditions of the BSD License

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
index 212dd51102..8aa568111f 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -3,7 +3,17 @@
   and volatile storage space and install variable architecture protocol

   based on SMM variable module.

 

-Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved.<BR>

+  Caution: This module requires additional review when modified.

+  This driver will have external input - variable data.

+  This external input must be validated carefully to avoid security issue like

+  buffer overflow, integer overflow.

+

+  RuntimeServiceGetVariable() and RuntimeServiceSetVariable() are external API

+  to receive data buffer. The size should be checked carefully.

+

+  InitCommunicateBuffer() is really function to check the variable data size.

+

+Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>

 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 full text of the license may be found at 

@@ -50,6 +60,9 @@ UINTN                            mVariableBufferSize;
   The communicate size is: SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE +

   DataSize.

 

+  Caution: This function may receive untrusted input.

+  The data size external input, so this function will validate it carefully to avoid buffer overflow.

+

   @param[out]      DataPtr          Points to the data in the communicate buffer.

   @param[in]       DataSize         The data size to send to SMM.

   @param[in]       Function         The function number to initialize the communicate header.

@@ -119,6 +132,9 @@ SendCommunicateBuffer (
 /**

   This code finds variable in storage blocks (Volatile or Non-Volatile).

 

+  Caution: This function may receive untrusted input.

+  The data size is external input, so this function will validate it carefully to avoid buffer overflow.

+

   @param[in]      VariableName       Name of Variable to be found.

   @param[in]      VendorGuid         Variable vendor GUID.

   @param[out]     Attributes         Attribute value of the variable found.

@@ -264,6 +280,9 @@ RuntimeServiceGetNextVariableName (
 /**

   This code sets variable in storage blocks (Volatile or Non-Volatile).

 

+  Caution: This function may receive untrusted input.

+  The data size and data are external input, so this function will validate it carefully to avoid buffer overflow.

+

   @param[in] VariableName                 Name of Variable to be found.

   @param[in] VendorGuid                   Variable vendor GUID.

   @param[in] Attributes                   Attribute value of the variable found

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.inf b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.inf
index c1fb6acae8..3842f0105d 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.inf
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.inf
@@ -5,7 +5,12 @@
 #  installs variable arch protocol and variable write arch protocol and works 

 #  with SMM variable module together. 

 #

-# Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved.<BR>

+#  Caution: This module requires additional review when modified.

+#  This driver will have external input - variable data.

+#  This external input must be validated carefully to avoid security issue like

+#  buffer overflow, integer overflow.

+#

+# Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>

 # 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 full text of the license may be found at