public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: chenc2 <chen.a.chen@intel.com>
To: edk2-devel@lists.01.org
Cc: chenc2 <chen.a.chen@intel.com>,
	Zhang Chao <chao.b.zhang@intel.com>, Wu Hao <hao.a.wu@intel.com>
Subject: [PATCH] SecurityPkg/SecureBootConfigDxe: Add check to avoid
Date: Wed, 18 Oct 2017 12:50:01 +0800	[thread overview]
Message-ID: <20171018045001.16012-1-chen.a.chen@intel.com> (raw)

The function HiiGetString will return NULL pointer when
the platform does not install the appropriate string or
call HiiGetString fail.(For example, HII not support specified
language.)

Cc: Zhang Chao <chao.b.zhang@intel.com>
Cc: Wu Hao <hao.a.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: chenc2 <chen.a.chen@intel.com>
---
 .../SecureBootConfigDxe/SecureBootConfigImpl.c     | 97 +++++++++++++++-------
 1 file changed, 66 insertions(+), 31 deletions(-)

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index acb0dc0558..4ce5172701 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -3572,6 +3572,9 @@ LoadSignatureList (
 {
   EFI_STATUS            Status;
   EFI_STRING_ID         ListType;
+  EFI_STRING            FormatNameString;
+  EFI_STRING            FormatHelpString;
+  EFI_STRING            FormatTypeString;
   EFI_SIGNATURE_LIST    *ListWalker;
   EFI_IFR_GUID_LABEL    *StartLabel;
   EFI_IFR_GUID_LABEL    *EndLabel;
@@ -3705,6 +3708,12 @@ LoadSignatureList (
     goto ON_EXIT;
   }
 
+  FormatNameString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_NAME_FORMAT), NULL);
+  FormatHelpString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_HELP_FORMAT), NULL);
+  if (FormatNameString == NULL || FormatHelpString == NULL) {
+    goto ON_EXIT;
+  }
+
   RemainingSize = DataSize;
   ListWalker    = (EFI_SIGNATURE_LIST *)VariableData;
   while ((RemainingSize > 0) && (RemainingSize >= ListWalker->SignatureListSize)) {
@@ -3725,21 +3734,23 @@ LoadSignatureList (
     } else {
       ListType = STRING_TOKEN (STR_LIST_TYPE_UNKNOWN);
     }
+    FormatTypeString = HiiGetString (PrivateData->HiiHandle, ListType, NULL);
+    if (FormatTypeString == NULL) {
+      goto ON_EXIT;
+    }
 
     ZeroMem (NameBuffer, sizeof (NameBuffer));
-    UnicodeSPrint (NameBuffer,
-      sizeof (NameBuffer),
-      HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_NAME_FORMAT), NULL),
-      Index + 1
-    );
+    UnicodeSPrint (NameBuffer, sizeof (NameBuffer), FormatNameString, Index + 1);
 
     ZeroMem (HelpBuffer, sizeof (HelpBuffer));
     UnicodeSPrint (HelpBuffer,
       sizeof (HelpBuffer),
-      HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_HELP_FORMAT), NULL),
-      HiiGetString (PrivateData->HiiHandle, ListType, NULL),
+      FormatHelpString,
+      FormatTypeString,
       SIGNATURE_DATA_COUNTS (ListWalker)
     );
+    SECUREBOOT_FREE_NON_NULL (FormatTypeString);
+    FormatTypeString = NULL;
 
     HiiCreateGotoOpCode (
       StartOpCodeHandle,
@@ -3777,6 +3788,8 @@ ON_EXIT:
   SECUREBOOT_FREE_NON_OPCODE (EndGotoHandle);
 
   SECUREBOOT_FREE_NON_NULL (VariableData);
+  SECUREBOOT_FREE_NON_NULL (FormatNameString);
+  SECUREBOOT_FREE_NON_NULL (FormatHelpString);
 
   PrivateData->ListCount = Index;
 
@@ -3922,6 +3935,8 @@ FormatHelpInfo (
   EFI_STATUS      Status;
   EFI_TIME        *Time;
   EFI_STRING_ID   ListTypeId;
+  EFI_STRING      FormatHelpString;
+  EFI_STRING      FormatTypeString;
   UINTN           DataSize;
   UINTN           HelpInfoIndex;
   UINTN           TotalSize;
@@ -3969,6 +3984,11 @@ FormatHelpInfo (
     goto ON_EXIT;
   }
 
+  FormatTypeString = HiiGetString (PrivateData->HiiHandle, ListTypeId, NULL);
+  if (FormatTypeString == NULL) {
+    goto ON_EXIT;
+  }
+
   TotalSize = 1024;
   HelpInfoString = AllocateZeroPool (TotalSize);
   if (HelpInfoString == NULL) {
@@ -3981,40 +4001,45 @@ FormatHelpInfo (
   //
   ZeroMem (GuidString, sizeof (GuidString));
   GuidToString(&DataEntry->SignatureOwner, GuidString, BUFFER_MAX_SIZE);
+  FormatHelpString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_GUID), NULL);
+  if (FormatHelpString == NULL) {
+    goto ON_EXIT;
+  }
   HelpInfoIndex += UnicodeSPrint (
                      &HelpInfoString[HelpInfoIndex],
                      TotalSize - sizeof(CHAR16) * HelpInfoIndex,
-                     HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_GUID), NULL),
+                     FormatHelpString,
                      GuidString
                    );
+  SECUREBOOT_FREE_NON_NULL (FormatHelpString);
+  FormatHelpString = NULL;
 
   //
   // Format content part, it depends on the type of signature list, hash value or CN.
   //
   if (IsCert) {
     GetCommonNameFromX509 (ListEntry, DataEntry, &DataString);
-    HelpInfoIndex += UnicodeSPrint(
-                       &HelpInfoString[HelpInfoIndex],
-                       TotalSize - sizeof(CHAR16) * HelpInfoIndex,
-                       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_CN), NULL),
-                       HiiGetString (PrivateData->HiiHandle, ListTypeId, NULL),
-                       DataSize,
-                       DataString
-                     );
+    FormatHelpString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_CN), NULL);
   } else {
     //
     //  Format hash value for each signature data entry.
     //
     ParseHashValue (ListEntry, DataEntry, &DataString);
-    HelpInfoIndex += UnicodeSPrint (
-                       &HelpInfoString[HelpInfoIndex],
-                       TotalSize - sizeof(CHAR16) * HelpInfoIndex,
-                       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_HASH), NULL),
-                       HiiGetString (PrivateData->HiiHandle, ListTypeId, NULL),
-                       DataSize,
-                       DataString
-                     );
+    FormatHelpString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_HASH), NULL);
+  }
+  if (FormatHelpString == NULL) {
+    goto ON_EXIT;
   }
+  HelpInfoIndex += UnicodeSPrint (
+                     &HelpInfoString[HelpInfoIndex],
+                     TotalSize - sizeof (CHAR16) * HelpInfoIndex,
+                     FormatHelpString,
+                     FormatTypeString,
+                     DataSize,
+                     DataString
+                   );
+  SECUREBOOT_FREE_NON_NULL (FormatHelpString);
+  FormatHelpString = NULL;
 
   //
   // Format revocation time part.
@@ -4032,13 +4057,18 @@ FormatHelpInfo (
       Time->Minute,
       Time->Second
     );
-
+    FormatHelpString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_TIME), NULL);
+    if (FormatHelpString == NULL) {
+      goto ON_EXIT;
+    }
     UnicodeSPrint (
       &HelpInfoString[HelpInfoIndex],
       TotalSize - sizeof (CHAR16) * HelpInfoIndex,
-      HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_TIME), NULL),
+      FormatHelpString,
       TimeString
     );
+    SECUREBOOT_FREE_NON_NULL (FormatHelpString);
+    FormatHelpString = NULL;
   }
 
   *StringId = HiiSetString (PrivateData->HiiHandle, 0, HelpInfoString, NULL);
@@ -4046,6 +4076,8 @@ ON_EXIT:
   SECUREBOOT_FREE_NON_NULL (DataString);
   SECUREBOOT_FREE_NON_NULL (HelpInfoString);
 
+  SECUREBOOT_FREE_NON_NULL (FormatTypeString);
+
   return Status;
 }
 
@@ -4076,6 +4108,7 @@ LoadSignatureData (
   EFI_IFR_GUID_LABEL    *StartLabel;
   EFI_IFR_GUID_LABEL    *EndLabel;
   EFI_STRING_ID         HelpStringId;
+  EFI_STRING            FormatNameString;
   VOID                  *StartOpCodeHandle;
   VOID                  *EndOpCodeHandle;
   UINTN                 DataSize;
@@ -4167,17 +4200,18 @@ LoadSignatureData (
     ListWalker = (EFI_SIGNATURE_LIST *)((UINT8 *)ListWalker + ListWalker->SignatureListSize);
   }
 
+  FormatNameString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_NAME_FORMAT), NULL);
+  if (FormatNameString == NULL) {
+    goto ON_EXIT;
+  }
+
   DataWalker = (EFI_SIGNATURE_DATA *)((UINT8 *)ListWalker + sizeof(EFI_SIGNATURE_LIST) + ListWalker->SignatureHeaderSize);
   for (Index = 0; Index < SIGNATURE_DATA_COUNTS(ListWalker); Index = Index + 1) {
     //
     // Format name buffer.
     //
     ZeroMem (NameBuffer, sizeof (NameBuffer));
-    UnicodeSPrint (NameBuffer,
-      sizeof (NameBuffer),
-      HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_NAME_FORMAT), NULL),
-      Index + 1
-    );
+    UnicodeSPrint (NameBuffer, sizeof (NameBuffer), FormatNameString, Index + 1);
 
     //
     // Format help info buffer.
@@ -4221,6 +4255,7 @@ ON_EXIT:
   SECUREBOOT_FREE_NON_OPCODE (EndOpCodeHandle);
 
   SECUREBOOT_FREE_NON_NULL (VariableData);
+  SECUREBOOT_FREE_NON_NULL (FormatNameString);
 
   return Status;
 }
-- 
2.13.2.windows.1



             reply	other threads:[~2017-10-18  4:46 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-18  4:50 chenc2 [this message]
2017-10-18  7:05 ` [PATCH] SecurityPkg/SecureBootConfigDxe: Add check to avoid Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171018045001.16012-1-chen.a.chen@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox