From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mx.groups.io with SMTP id smtpd.web10.7956.1670571849012196270 for ; Thu, 08 Dec 2022 23:44:09 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=cAvIB2nm; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: jiewen.yao@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1670571849; x=1702107849; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=oLgoctXwUIB0Mrl8lR1RPobD+zieFzKofYqdHKfhvE8=; b=cAvIB2nm2BDrE6kmeSHUgGRztiEJNY8w6bV/bVMo0Q96FLS4uFdqIl+M SnEayDGs9KzrU0Jcs/wogsEF1BxA/o+pc5AUis3lE6Z1GeYggClZOITja qu/Tf9t1ujinYR9kT9ef05/4F3SDr/yCreKd23/1P0ublDMjzjl9vBNnE 1FdvIE98xoW9OMLOu9SrQuqrj6T/6vz4IICJcuhygt1qtbN476zeaYnqv hUe45j68GiLa05wcleq3MDHQ9pb2KZ5LdUuhcb+cmHD3AonNWusV7iCuB XNOv5aCSjmoym/ZLoH0T+ZfpLuGV+PWpBLyu5ik16uGjeuXpoIoUeDjQn g==; X-IronPort-AV: E=McAfee;i="6500,9779,10555"; a="300825517" X-IronPort-AV: E=Sophos;i="5.96,230,1665471600"; d="scan'208";a="300825517" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Dec 2022 23:43:59 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10555"; a="892570827" X-IronPort-AV: E=Sophos;i="5.96,230,1665471600"; d="scan'208";a="892570827" Received: from fmsmsx602.amr.corp.intel.com ([10.18.126.82]) by fmsmga006.fm.intel.com with ESMTP; 08 Dec 2022 23:43:59 -0800 Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Thu, 8 Dec 2022 23:43:58 -0800 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16 via Frontend Transport; Thu, 8 Dec 2022 23:43:58 -0800 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.169) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Thu, 8 Dec 2022 23:43:58 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=IdGgMiWlOm8MLb2+j8/o8Pj8H7p7lKbsRMNVG4Gs3SIxAgvntCmXow1kbzyeDUSlU33LLrhVISyfwzBHKb+y/wqZkTq35h5uLYlshkQODfzTx0/j2pKTwOcd12GPvaSOjk9B/EsDjownQGGNcjQVQI02DIkDI7J7D1CQhU+YbdMjs0lW7UW0XACnJPoEEryWPbQNX/MvLfv9XSEjqH1d4RKJM5Qkhv0FSdEWgMOqqNT7hheNdtqEeigwM6ESIU5xe0UedLuNu3N1qKFpbT1waW7mJRbxrRw3q2opdTG5ZNOQAlrMRMqp8dl5QzAZjf87q1TKRQbBopfya7M3WG+jFw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=GDY9IWIdKruwwwTbsR3DRZaxdRBt7XSMV7hZ23Lw/K4=; b=WWG8wZuyFLQw5Zes3OW3IQWyWtk0GaxgOsovC8yqSJBcONILHG7V3zyvM+JUcMM1diPpl+kYPQuyLRA6jrlptHel+m4DjIUCUY022jyblDN5y9NzjVePfSVjZFtfrMr6zToApy9KyeuZMI8dz/+heW2UrEDJxyiwdiDMs4iGDG5Km46B5MQ0JIKEnq/EOdyymKuenYFKZaazKj5bAvdZ+wZGIpo9c9h0/sY+aANWvN4SRbaXjS8/aQNNm8VuAl9y5thbVzqqVydOuu9i9VAo6/UQ3e2gmRYsBo6Ohp6LOzvcxkvYB6CkZrMOtQ0hFtw1oSt8FFNxlVFQaaHkoDzLrw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from PH0PR11MB5879.namprd11.prod.outlook.com (2603:10b6:510:142::5) by DS0PR11MB6399.namprd11.prod.outlook.com (2603:10b6:8:c8::5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5880.14; Fri, 9 Dec 2022 07:43:56 +0000 Received: from PH0PR11MB5879.namprd11.prod.outlook.com ([fe80::dfc:656b:a06b:72f]) by PH0PR11MB5879.namprd11.prod.outlook.com ([fe80::dfc:656b:a06b:72f%3]) with mapi id 15.20.5880.016; Fri, 9 Dec 2022 07:43:55 +0000 From: "Yao, Jiewen" To: Jon Maloy , "devel@edk2.groups.io" CC: "kraxel@redhat.com" , "lersek@redhat.com" , "Wang, Jian J" , "Xu, Min M" , "Yao, Jiewen" Subject: Re: [edk2-devel] [PATCH] SecurityPkg: check return value of GetEfiGlobalVariable2() in DxeImageVerificationHandler() Thread-Topic: [edk2-devel] [PATCH] SecurityPkg: check return value of GetEfiGlobalVariable2() in DxeImageVerificationHandler() Thread-Index: AQHZBfsIbt1oJNqz8kioRRPmdcrIba5lN7cA Date: Fri, 9 Dec 2022 07:43:55 +0000 Message-ID: References: <20221202030526.71113-1-jmaloy@redhat.com> In-Reply-To: <20221202030526.71113-1-jmaloy@redhat.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: PH0PR11MB5879:EE_|DS0PR11MB6399:EE_ x-ms-office365-filtering-correlation-id: 2169a00d-99cc-4a92-955b-08dad9b91fd2 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: rStHpuSA4/rgmWldDeuG6+wRUb5RQyyx9FRumOb/uVnV/pNxvR0QosizzrDiVpMo4/2uUrk1xcjzhgUD/RVFItLPbFi154ui0snsQUCl1oEs4h4mvugEN2x5hafRVHy2UXmV/KvnyZj9dat8/S63kBhIN9sHj1zz0+zHDBmBoGIEnTApUQyCvKJUSp9pzw3BDvf8X7RBDFJIWDAstO/8heCf5DOGP0vHehXi8Dk7DJ0z0Sbu5JioYq9buCXIrQ3SmgCJCxXDgmIk3TUz9p5aGhpVfsOM3yzknSmN+K8TG48HeYnWMWzHMca0NYCwlfk426ILGLWHygpdiAOxaSmoM04ZMyOF6X6AV21hAcFljk+h4EpdfNomv8jXSkf72kOH5kcGSpVYigRvuiUPtL893JvSDDMdfVzDPyaQ8NNS3EYK1ea4QWIJJvxUfPIbRtXbW+Q8IfzixfQk2W4O//cdC65yzNhyUl4hhB4YSt/tRlLGgaSYIV+OvW/FGrT41mBc+o8L86j30STw9Fp9X6qhvMqnRmSaRPa6sgtmSjJdu7BZU31LT9S+UknLsQL+cd7vAiXu5MP3Nxy2QAchKBXshJI3JMVWB3miXdpciTsId24F3q3NR1/t+rhSEuwD+aMWQdpjVjVGHumfnJtw5KDGRP9p2jW6ENIPyRWD3b51dhh+Q8oZutvZv85+zFIbAK/EhHC8D/VEUMyHgzZRbax5pw== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:PH0PR11MB5879.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(366004)(376002)(346002)(136003)(396003)(39860400002)(451199015)(8676002)(71200400001)(4326008)(41300700001)(82960400001)(122000001)(38100700002)(76116006)(2906002)(66946007)(38070700005)(52536014)(15650500001)(83380400001)(66446008)(107886003)(6506007)(55016003)(26005)(9686003)(7696005)(53546011)(5660300002)(478600001)(110136005)(19627235002)(66476007)(186003)(66556008)(8936002)(33656002)(86362001)(64756008)(54906003)(316002);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?oX/2mXfHTeD71xFPCSkSwedCwv7QMTFTFVTR61duX4AKvYH6X6v2FGi9U7bP?= =?us-ascii?Q?nzwFrO4soomk4AeUhiTXnh3GDYvCZy++5p/XWjEVM5f3ivrPmlC7eHF4B8ST?= =?us-ascii?Q?Y9aaRhfvDH/eSPwlOkfJfI255rv170dLfiAG323fqMbnW5bi8Wv1uoBpnjhC?= =?us-ascii?Q?q5UpBi9zPBMhL0pFcAcXN35UFVkic29PvxcfJzO0s3nZA3tBDD+hQ1Jb3ev4?= =?us-ascii?Q?GKa30FVFQJthsqSuxQRhGyC+6M/oM5AuRmVhgMrb7F08zmBTndN8D8r/SFnJ?= =?us-ascii?Q?qY739JVbfa1Q1Tq9i2coBZjeMsmgGQcVfrb296vXWvslLIfEMv8meadwrKug?= =?us-ascii?Q?xlElP/TxzOsKRsnuTeAk/u0Dl/VIXFk/rC65KicNebk0mPBFlvU/y31vt4oC?= =?us-ascii?Q?+DOYSpA0ZGL+jOEy2QsH5NeXWa0ZnR9AMZjPfe/8swK3Nms/gK/7IzFotEls?= =?us-ascii?Q?GbsGlQ16kNzHPlz9MHrd3llmDl1NHuZAVZtWB7esiM3SahkxggvAqaAYT+4R?= =?us-ascii?Q?RrJ2WaQx836lef8ig8pWQm6hSRoSqC5YVEUEJ27mXJNxGoMti7lFS5IAw8UH?= =?us-ascii?Q?rn2Fej9EqLFMHL844/s1ckl52Jqat80SjQwoa3Ln2ZYQTEoJuAO613acwqzt?= =?us-ascii?Q?ZUNShyb4cpDWST0Y4Hk4Ud9YBCzfbq3hCUkS6jxNRUou6xh5eZU6AXEMxJ2G?= =?us-ascii?Q?YUVN68pGc2zXFgxU/ZwU0Pp36nxr7zROSd88j5VOW3+GmwOuZsDZ1f9thTsz?= =?us-ascii?Q?9Lcfh5U1sOABMN1GFsRbnYHbwRpVukTNfCmeStZmyH6kB0mttC0HlPtX2eOI?= =?us-ascii?Q?tbplyHVlYvILvL/6TCZ/o+DcM5Lap5p3ObIEWX6yQhYB9r/1dGmtOxdCU+rK?= =?us-ascii?Q?Yec4mzkHS4//o3WIe4M9nCukWijJKhgq2YJS9sOrlz/Fisod8iituNW7Y8r4?= =?us-ascii?Q?rPQTle2IOa8WLTdkSv3LFClzXPl8fPvQJn10Lz3T3JcLrldBgh6tGfwpIdIW?= =?us-ascii?Q?4f4gS9KDeTnwh3fsWX28vclOPxpnvkNEIFoE2oapBhR/oRL/m3MgLgGnHypU?= =?us-ascii?Q?3bzCWUqNdvrHDBu+VWssCNhBRxyMQ+ANY0wPwxC6HP9q+cXv/z5gaPaH1WuD?= =?us-ascii?Q?/1TsPoIXBOS14NzXG45PHbz900P+LXaw2zy0Y0EpoWedR0jK+uApeQxjxmVP?= =?us-ascii?Q?VpHNXatoe4FDHw+Ya+R8Q4MI4NOAGMhgFBhr5yPn3jpvETywnMQxjpBrvwDL?= =?us-ascii?Q?LdaWvyixTDjp1GbpXVuESBe6WIJDRtwpZWqZT9r22oEYZTU+foRECOTg8xok?= =?us-ascii?Q?wilGC/0Rx5+g0GH+ywxM1gThUnew1+zrpojDG7mnpxLz+7EPSa+rw8mAiXPU?= =?us-ascii?Q?+4m4xYwXPgn52pLf7FKAKOAwwSDKkggguzTi7kRM97ggjGR0P22aBLRLCLQY?= =?us-ascii?Q?BtlXhkONxEwBALL0hIWXzRmiTeddmp66bsGIVUmKacpW44jASxD8tYBg0aP9?= =?us-ascii?Q?Tw5LDtZSQ+tC+85R0Rp2cyfu1lrV5jeYO/itSTH4D7mniYSMbjBx+dsi9DvX?= =?us-ascii?Q?lw8b9FoTa/A+aMCbPpWcVuibtNCT0jSxAyZLiSWC?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5879.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 2169a00d-99cc-4a92-955b-08dad9b91fd2 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Dec 2022 07:43:55.2106 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 4BvXv6UI8uZ5/7Z0920gZUSVxojtpZ3yj8kfcGUSo61Sedny9kFP/+JZVwRJFBefSWAu0BU4taeVHpNr17NP6A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB6399 Return-Path: jiewen.yao@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks for the patch. Would you please share the info that, how this patch is tested? Have you created a situation to cause GetEfiGlobalVariable2() returns failu= re here? > -----Original Message----- > From: Jon Maloy > Sent: Friday, December 2, 2022 11:05 AM > To: devel@edk2.groups.io > Cc: kraxel@redhat.com; lersek@redhat.com; jmaloy@redhat.com; Yao, > Jiewen ; Wang, Jian J ; Xu, > Min M > Subject: [edk2-devel] [PATCH] SecurityPkg: check return value of > GetEfiGlobalVariable2() in DxeImageVerificationHandler() >=20 > Fixes: CVE-2019-14560 >=20 > GetEfiGlobalVariable2() is used in some instances when looking up the > SecureBoot UEFI variable. The API can fail in certain circumstances, > for example, if AllocatePool() fails or if gRT->GetVariable() fails. > In the case of secure boot checks, it is critical that this return value > is checked. if an attacker can cause the API to fail, it would currently > constitute a secure boot bypass. >=20 > This return value check is missing in the function > DxeImageVerificationHandler(), > so we add it here. >=20 > This commit is almost identical to one suggested by Jian J Wang > > on 2019-09-09, but that one was for some reason never posted to the > edk2-devel > list. We now make a new attempt to get it reviewed and applied. >=20 > Cc: Jiewen Yao > Cc: Jian J Wang > Cc: Min Xu > Cc: devel@edk2.groups.io >=20 > Signed-off-by: Jon Maloy > --- > .../DxeImageVerificationLib.c | 39 +++++++++++-------- > 1 file changed, 23 insertions(+), 16 deletions(-) >=20 > diff --git > a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c > b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c > index 66e2f5eaa3c0..4ae0bd8b20db 100644 > --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib= .c > +++ > b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c > @@ -1686,6 +1686,7 @@ DxeImageVerificationHandler ( > RETURN_STATUS PeCoffStatus; > EFI_STATUS HashStatus; > EFI_STATUS DbStatus; > + EFI_STATUS SecBootStatus; > BOOLEAN IsFound; >=20 > SignatureList =3D NULL; > @@ -1742,23 +1743,29 @@ DxeImageVerificationHandler ( > CpuDeadLoop (); > } >=20 > - GetEfiGlobalVariable2 (EFI_SECURE_BOOT_MODE_NAME, (VOID > **)&SecureBoot, NULL); > - // > - // Skip verification if SecureBoot variable doesn't exist. > - // > - if (SecureBoot =3D=3D NULL) { > - return EFI_SUCCESS; > - } > + SecBootStatus =3D GetEfiGlobalVariable2 > (EFI_SECURE_BOOT_MODE_NAME, (VOID **)&SecureBoot, NULL); > + if (!EFI_ERROR (SecBootStatus)) { > + if (SecureBoot =3D=3D NULL) { > + // > + // Skip verification if SecureBoot variable doesn't exist. > + // > + return EFI_SUCCESS; > + } else { > + // > + // Skip verification if SecureBoot is disabled but not AuditMode > + // > + if (*SecureBoot =3D=3D SECURE_BOOT_MODE_DISABLE) { > + FreePool (SecureBoot); > + return EFI_SUCCESS; > + } > + FreePool (SecureBoot); > + } > + } else { > + // > + // Assume SecureBoot enabled in the case of error. > + // > + } >=20 > - // > - // Skip verification if SecureBoot is disabled but not AuditMode > - // > - if (*SecureBoot =3D=3D SECURE_BOOT_MODE_DISABLE) { > - FreePool (SecureBoot); > - return EFI_SUCCESS; > - } > - > - FreePool (SecureBoot); >=20 > // > // Read the Dos header. > -- > 2.35.3