From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.65; helo=mga03.intel.com; envelope-from=chao.b.zhang@intel.com; receiver=edk2-devel@lists.01.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id CAC7821E7821E for ; Wed, 18 Oct 2017 21:26:38 -0700 (PDT) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Oct 2017 21:30:16 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,399,1503385200"; d="scan'208";a="324982514" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga004.fm.intel.com with ESMTP; 18 Oct 2017 21:30:15 -0700 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 18 Oct 2017 21:30:15 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 18 Oct 2017 21:30:14 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.175]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.213]) with mapi id 14.03.0319.002; Thu, 19 Oct 2017 12:30:13 +0800 From: "Zhang, Chao B" To: "Chen, Chen A" , "edk2-devel@lists.01.org" CC: "Wu, Hao A" Thread-Topic: [PATCH v2] SecurityPkg/SecureBootConfigDxe: Handle lack of STR_SIGNATURE_* tokens Thread-Index: AQHTR+fynKp7L7MwV0+ZegSk/x75FqLqlaIA Date: Thu, 19 Oct 2017 04:30:12 +0000 Message-ID: References: <20171018080548.5608-1-chen.a.chen@intel.com> In-Reply-To: <20171018080548.5608-1-chen.a.chen@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2] SecurityPkg/SecureBootConfigDxe: Handle lack of STR_SIGNATURE_* tokens X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Oct 2017 04:26:40 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Chao Zhang -----Original Message----- From: Chen, Chen A=20 Sent: Wednesday, October 18, 2017 4:06 PM To: edk2-devel@lists.01.org Cc: Chen, Chen A ; Zhang, Chao B ; Wu, Hao A Subject: [PATCH v2] SecurityPkg/SecureBootConfigDxe: Handle lack of STR_SIG= NATURE_* tokens Add check to avoid NULL ptr dereference. The function HiiGetString will ret= urn 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 Cc: Wu Hao Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: chenc2 --- .../SecureBootConfigDxe/SecureBootConfigImpl.c | 113 ++++++++++++++---= ---- 1 file changed, 76 insertions(+), 37 deletions(-) diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBo= otConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/Secu= reBootConfigImpl.c index acb0dc0558..d035763106 100644 --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfi= gImpl.c +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo +++ nfigImpl.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; @@ -3591,6 +3594,8 @@ LoadSignatureList ( CHAR16 HelpBuffer[BUFFER_MAX_SIZE]; =20 Status =3D EFI_SUCCESS; + FormatNameString =3D NULL; + FormatHelpString =3D NULL; StartOpCodeHandle =3D NULL; EndOpCodeHandle =3D NULL; StartGotoHandle =3D NULL; @@ -3705,6 +3710,12 @@ LoadSignatureList ( goto ON_EXIT; } =20 + FormatNameString =3D HiiGetString (PrivateData->HiiHandle, STRING_TOKEN= =20 + (STR_SIGNATURE_LIST_NAME_FORMAT), NULL); FormatHelpString =3D=20 + HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_HE= LP_FORMAT), NULL); if (FormatNameString =3D=3D NULL || FormatHelpString = =3D=3D NULL) { + goto ON_EXIT; + } + RemainingSize =3D DataSize; ListWalker =3D (EFI_SIGNATURE_LIST *)VariableData; while ((RemainingSize > 0) && (RemainingSize >=3D ListWalker->SignatureL= istSize)) { @@ -3725,21 +3736,23 @@ LoadSignatureList ( } else { ListType =3D STRING_TOKEN (STR_LIST_TYPE_UNKNOWN); } + FormatTypeString =3D HiiGetString (PrivateData->HiiHandle, ListType, N= ULL); + if (FormatTypeString =3D=3D NULL) { + goto ON_EXIT; + } =20 ZeroMem (NameBuffer, sizeof (NameBuffer)); - UnicodeSPrint (NameBuffer, - sizeof (NameBuffer), - HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LI= ST_NAME_FORMAT), NULL), - Index + 1 - ); + UnicodeSPrint (NameBuffer, sizeof (NameBuffer), FormatNameString,=20 + Index + 1); =20 ZeroMem (HelpBuffer, sizeof (HelpBuffer)); UnicodeSPrint (HelpBuffer, sizeof (HelpBuffer), - HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LI= ST_HELP_FORMAT), NULL), - HiiGetString (PrivateData->HiiHandle, ListType, NULL), + FormatHelpString, + FormatTypeString, SIGNATURE_DATA_COUNTS (ListWalker) ); + SECUREBOOT_FREE_NON_NULL (FormatTypeString); + FormatTypeString =3D NULL; =20 HiiCreateGotoOpCode ( StartOpCodeHandle, @@ -3777,6 +3790,8 @@ ON_EXIT: SECUREBOOT_FREE_NON_OPCODE (EndGotoHandle); =20 SECUREBOOT_FREE_NON_NULL (VariableData); + SECUREBOOT_FREE_NON_NULL (FormatNameString); =20 + SECUREBOOT_FREE_NON_NULL (FormatHelpString); =20 PrivateData->ListCount =3D Index; =20 @@ -3922,6 +3937,8 @@ FormatHelpInfo ( EFI_STATUS Status; EFI_TIME *Time; EFI_STRING_ID ListTypeId; + EFI_STRING FormatHelpString; + EFI_STRING FormatTypeString; UINTN DataSize; UINTN HelpInfoIndex; UINTN TotalSize; @@ -3931,12 +3948,13 @@ FormatHelpInfo ( CHAR16 *HelpInfoString; BOOLEAN IsCert; =20 - Status =3D EFI_SUCCESS; - Time =3D NULL; - HelpInfoIndex =3D 0; - DataString =3D NULL; - HelpInfoString =3D NULL; - IsCert =3D FALSE; + Status =3D EFI_SUCCESS; + Time =3D NULL; + FormatTypeString =3D NULL; + HelpInfoIndex =3D 0; + DataString =3D NULL; + HelpInfoString =3D NULL; + IsCert =3D FALSE; =20 if (CompareGuid(&ListEntry->SignatureType, &gEfiCertRsa2048Guid)) { ListTypeId =3D STRING_TOKEN(STR_LIST_TYPE_RSA2048_SHA256); @@ -3969,6 +3987,11 @@ FormatHelpInfo ( goto ON_EXIT; } =20 + FormatTypeString =3D HiiGetString (PrivateData->HiiHandle, ListTypeId,=20 + NULL); if (FormatTypeString =3D=3D NULL) { + goto ON_EXIT; + } + TotalSize =3D 1024; HelpInfoString =3D AllocateZeroPool (TotalSize); if (HelpInfoString =3D=3D NULL) { @@ -3981,40 +4004,45 @@ FormatHelpInfo ( // ZeroMem (GuidString, sizeof (GuidString)); GuidToString(&DataEntry->SignatureOwner, GuidString, BUFFER_MAX_SIZE); + FormatHelpString =3D HiiGetString (PrivateData->HiiHandle, STRING_TOKEN= =20 + (STR_SIGNATURE_DATA_HELP_FORMAT_GUID), NULL); if (FormatHelpString =3D= =3D NULL) { + goto ON_EXIT; + } HelpInfoIndex +=3D UnicodeSPrint ( &HelpInfoString[HelpInfoIndex], TotalSize - sizeof(CHAR16) * HelpInfoIndex, - HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (S= TR_SIGNATURE_DATA_HELP_FORMAT_GUID), NULL), + FormatHelpString, GuidString ); + SECUREBOOT_FREE_NON_NULL (FormatHelpString); FormatHelpString =3D=20 + NULL; =20 // // Format content part, it depends on the type of signature list, hash v= alue or CN. // if (IsCert) { GetCommonNameFromX509 (ListEntry, DataEntry, &DataString); - HelpInfoIndex +=3D UnicodeSPrint( - &HelpInfoString[HelpInfoIndex], - TotalSize - sizeof(CHAR16) * HelpInfoIndex, - HiiGetString (PrivateData->HiiHandle, STRING_TOKEN = (STR_SIGNATURE_DATA_HELP_FORMAT_CN), NULL), - HiiGetString (PrivateData->HiiHandle, ListTypeId, N= ULL), - DataSize, - DataString - ); + FormatHelpString =3D HiiGetString (PrivateData->HiiHandle,=20 + STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_CN), NULL); } else { // // Format hash value for each signature data entry. // ParseHashValue (ListEntry, DataEntry, &DataString); - HelpInfoIndex +=3D UnicodeSPrint ( - &HelpInfoString[HelpInfoIndex], - TotalSize - sizeof(CHAR16) * HelpInfoIndex, - HiiGetString (PrivateData->HiiHandle, STRING_TOKEN = (STR_SIGNATURE_DATA_HELP_FORMAT_HASH), NULL), - HiiGetString (PrivateData->HiiHandle, ListTypeId, N= ULL), - DataSize, - DataString - ); + FormatHelpString =3D HiiGetString (PrivateData->HiiHandle,=20 + STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_HASH), NULL); } if=20 + (FormatHelpString =3D=3D NULL) { + goto ON_EXIT; } + HelpInfoIndex +=3D UnicodeSPrint ( + &HelpInfoString[HelpInfoIndex], + TotalSize - sizeof (CHAR16) * HelpInfoIndex, + FormatHelpString, + FormatTypeString, + DataSize, + DataString + ); + SECUREBOOT_FREE_NON_NULL (FormatHelpString); FormatHelpString =3D=20 + NULL; =20 // // Format revocation time part. @@ -4032,13 +4060,18 @@ FormatHelpInfo ( Time->Minute, Time->Second ); - + FormatHelpString =3D HiiGetString (PrivateData->HiiHandle, STRING_TOKE= N (STR_SIGNATURE_DATA_HELP_FORMAT_TIME), NULL); + if (FormatHelpString =3D=3D NULL) { + goto ON_EXIT; + } UnicodeSPrint ( &HelpInfoString[HelpInfoIndex], TotalSize - sizeof (CHAR16) * HelpInfoIndex, - HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DA= TA_HELP_FORMAT_TIME), NULL), + FormatHelpString, TimeString ); + SECUREBOOT_FREE_NON_NULL (FormatHelpString); + FormatHelpString =3D NULL; } =20 *StringId =3D HiiSetString (PrivateData->HiiHandle, 0, HelpInfoString, N= ULL); @@ -4046,6 +4079,8 @@ ON_EXIT: SECUREBOOT_FREE_NON_NULL (DataString); SECUREBOOT_FREE_NON_NULL (HelpInfoString); =20 + SECUREBOOT_FREE_NON_NULL (FormatTypeString); + return Status; } =20 @@ -4076,6 +4111,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; @@ -4086,6 +4122,7 @@ LoadSignatureData ( CHAR16 NameBuffer[BUFFER_MAX_SIZE]; =20 Status =3D EFI_SUCCESS; + FormatNameString =3D NULL; StartOpCodeHandle =3D NULL; EndOpCodeHandle =3D NULL; Index =3D 0; @@ -4167,17 +4204,18 @@ LoadSignatureData ( ListWalker =3D (EFI_SIGNATURE_LIST *)((UINT8 *)ListWalker + ListWalker= ->SignatureListSize); } =20 + FormatNameString =3D HiiGetString (PrivateData->HiiHandle, STRING_TOKEN= =20 + (STR_SIGNATURE_DATA_NAME_FORMAT), NULL); if (FormatNameString =3D=3D NUL= L) { + goto ON_EXIT; + } + DataWalker =3D (EFI_SIGNATURE_DATA *)((UINT8 *)ListWalker + sizeof(EFI_S= IGNATURE_LIST) + ListWalker->SignatureHeaderSize); for (Index =3D 0; Index < SIGNATURE_DATA_COUNTS(ListWalker); Index =3D I= ndex + 1) { // // Format name buffer. // ZeroMem (NameBuffer, sizeof (NameBuffer)); - UnicodeSPrint (NameBuffer, - sizeof (NameBuffer), - HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DA= TA_NAME_FORMAT), NULL), - Index + 1 - ); + UnicodeSPrint (NameBuffer, sizeof (NameBuffer), FormatNameString,=20 + Index + 1); =20 // // Format help info buffer. @@ -4221,6 +4259,7 @@ ON_EXIT: SECUREBOOT_FREE_NON_OPCODE (EndOpCodeHandle); =20 SECUREBOOT_FREE_NON_NULL (VariableData); + SECUREBOOT_FREE_NON_NULL (FormatNameString); =20 return Status; } -- 2.13.2.windows.1