SecurityPkg/VariableAuthenticated: Check if there is a NV Variable Storage header...

The Variable PEI and RuntimeDxe drivers were using the attribute 'HeaderLength' of
EFI_FIRMWARE_VOLUME_HEADER without checking if a Firmware Volume Header was existing at
the base address.
In case the Firmware Volume Header does not exist or is corrupted, the attribute 'HeaderLength'
is a non valid value that can lead to a non valid physical address when accessing produces an
access error.

Signed-off-by: oliviermartin
Reviewed-by: rsun3
Reviewed-by: niruiyu
git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@12845 6f19259b-4bc3-4df7-8a09-765794883524

diff --git a/SecurityPkg/VariableAuthenticated/Pei/Variable.c b/SecurityPkg/VariableAuthenticated/Pei/Variable.c
index 7549be2dca45ca02930b5bad5d02c9767652afc7..d27f679073827937fb6a57d763edd8903e0fc1d3 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/Pei/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/Pei/Variable.c
@@ -359,6 +359,15 @@ GetVariableStore (
                                                            PcdGet64 (PcdFlashNvStorageVariableBase64) : \r
                                                            PcdGet32 (PcdFlashNvStorageVariableBase)\r
                                                           );\r
+\r
+        //\r
+        // Check if the Firmware Volume is not corrupted\r
+        //\r
+        if ((FvHeader->Signature != EFI_FVH_SIGNATURE) || (!CompareGuid (&gEfiSystemNvDataFvGuid, &FvHeader->FileSystemGuid))) {\r
+          DEBUG ((EFI_D_ERROR, "Firmware Volume for Variable Store is corrupted\n"));\r
+          break;\r
+        }\r
+        \r
         VariableStoreHeader = (VARIABLE_STORE_HEADER *) ((UINT8 *) FvHeader + FvHeader->HeaderLength);\r
 \r
         if (IndexTable != NULL) {\r

diff --git a/SecurityPkg/VariableAuthenticated/Pei/Variable.h b/SecurityPkg/VariableAuthenticated/Pei/Variable.h
index 75d32dac5f8e664709ac94a24fa59c8aed8218ee..a85d3bbab65dd4e58cf16b025f30d40f33dfd7d4 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/Pei/Variable.h
+++ b/SecurityPkg/VariableAuthenticated/Pei/Variable.h
@@ -29,6 +29,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 \r
 #include <Guid/AuthenticatedVariableFormat.h>\r
 #include <Guid/VariableIndexTable.h>\r
+#include <Guid/SystemNvDataGuid.h>\r
 \r
 typedef enum {\r
   VariableStoreTypeHob,\r

diff --git a/SecurityPkg/VariableAuthenticated/Pei/VariablePei.inf b/SecurityPkg/VariableAuthenticated/Pei/VariablePei.inf
index 7863293ff820f4dd830f9ad1af6b2501c2beeb84..e74143cd19c5151e7e7b3470b0d8fe4fab7c1861 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/Pei/VariablePei.inf
+++ b/SecurityPkg/VariableAuthenticated/Pei/VariablePei.inf
@@ -46,6 +46,7 @@
 [Guids]\r
   gEfiAuthenticatedVariableGuid\r
   gEfiVariableIndexTableGuid\r
+  gEfiSystemNvDataFvGuid\r
 \r
 [Ppis]\r
   gEfiPeiReadOnlyVariable2PpiGuid                ## SOMETIMES_PRODUCES (Not for boot mode RECOVERY)\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
index 7d0d21502af63e9383625168f8ce6cf45aa91902..e3fc48b497cfc64b470808d96fb78f168c3d477f 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
@@ -1157,7 +1157,7 @@ VariableGetBestLanguage (
 \r
 **/\r
 VOID\r
-AutoUpdateLangVariable(\r
+AutoUpdateLangVariable (\r
   IN  CHAR16             *VariableName,\r
   IN  VOID               *Data,\r
   IN  UINTN              DataSize\r
@@ -2616,6 +2616,17 @@ VariableCommonInitialize (
   if (TempVariableStoreHeader == 0) {\r
     TempVariableStoreHeader = (EFI_PHYSICAL_ADDRESS) PcdGet32 (PcdFlashNvStorageVariableBase);\r
   }\r
+  \r
+  //\r
+  // Check if the Firmware Volume is not corrupted\r
+  //\r
+  if ((((EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)(TempVariableStoreHeader))->Signature != EFI_FVH_SIGNATURE) ||\r
+      (!CompareGuid (&gEfiSystemNvDataFvGuid, &((EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)(TempVariableStoreHeader))->FileSystemGuid))) {\r
+    Status = EFI_VOLUME_CORRUPTED;\r
+    DEBUG ((EFI_D_ERROR, "Firmware Volume for Variable Store is corrupted\n"));\r
+    goto Done;\r
+  }\r
+\r
   VariableStoreBase       = TempVariableStoreHeader + \\r
                               (((EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)(TempVariableStoreHeader)) -> HeaderLength);\r
   VariableStoreLength     = (UINT64) PcdGet32 (PcdFlashNvStorageVariableSize) - \\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.h b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.h
index 91c7b4aac0142dbc982b35a6cfe42bb18ba0c536..58d1e5a8cdab4fe2bcef7bdfeb7fe4b79de2a7a8 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.h
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.h
@@ -39,6 +39,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Guid/EventGroup.h>\r
 #include <Guid/AuthenticatedVariableFormat.h>\r
 #include <Guid/ImageAuthentication.h>\r
+#include <Guid/SystemNvDataGuid.h>\r
 \r
 #define VARIABLE_RECLAIM_THRESHOLD (1024)\r
 \r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableRuntimeDxe.inf b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableRuntimeDxe.inf
index 7fcb640de038e0eda200adc11ada5eca5c8d5087..70717c457331424114e99638fe86def236708dbe 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableRuntimeDxe.inf
@@ -73,6 +73,7 @@
   gEfiCertPkcs7Guid\r
   gEfiCertRsa2048Guid\r
   gEfiSecureBootEnableDisableGuid\r
+  gEfiSystemNvDataFvGuid                        ## CONSUMES\r
 \r
 [Pcd]\r
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf
index 628c9829fdf6d24de8cab7b7338a09feeb8cf50f..84762dc406a73389792cabbd2430b8db0081f64f 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmm.inf
@@ -78,6 +78,7 @@
   gEfiCertPkcs7Guid\r
   gEfiCertRsa2048Guid\r
   gEfiSecureBootEnableDisableGuid\r
+  gEfiSystemNvDataFvGuid                        ## CONSUMES\r
 \r
 [Pcd]\r
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize\r