From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mx.groups.io with SMTP id smtpd.web10.12002.1599011750050930161 for ; Tue, 01 Sep 2020 18:55:50 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=ZcPVbd6P; spf=pass (domain: intel.com, ip: 134.134.136.100, mailfrom: min.m.xu@intel.com) IronPort-SDR: HHITqS8V7wTC3CvtUVAsRiEXljpiQdKtEDVdxRD4LC0rUZ74hiqihP2n0dTxDzWL7z2qgzZoRX MY3NrAc/M5tw== X-IronPort-AV: E=McAfee;i="6000,8403,9731"; a="221521697" X-IronPort-AV: E=Sophos;i="5.76,381,1592895600"; d="scan'208";a="221521697" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Sep 2020 18:55:48 -0700 IronPort-SDR: rshs2UdlC/MLFC4pe1yJhr/kkMaHeFAZwcUlLNBgrGLnRyofUdSjRdKocCUQQ5ogHMPpiI/bNB tH5/jfed8Qtg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.76,381,1592895600"; d="scan'208";a="333933157" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by fmsmga002.fm.intel.com with ESMTP; 01 Sep 2020 18:55:48 -0700 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 1 Sep 2020 18:55:47 -0700 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5 via Frontend Transport; Tue, 1 Sep 2020 18:55:47 -0700 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.108) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Tue, 1 Sep 2020 18:55:37 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fbawTjdKKdon8dDvaVqueFnpE0NyvFMTrUCFs3POnwnshaQE7aVk5/D+d35/1CJKqqe732QHjU5H5ZjrMiKpXasaJGi01oiVGXc7zNH4/nU4ONsM6/OSI9wE4v8bnr55MMQqrOnzKg536pYkIE1OUBveqqYf3N7ss5N0ipROC3fQP/hTwhJA1F42ztH3QHmeOWjf5vrnQHPoujAFZvjR7tpOWUc56TTS4WoBueam6NSmqf8lCVtlGPEMJsw3/X+7q8Rp2ebTDDbOGyj7C/uBWTh/Bo3fNi5WyF0rIDA2efl7dR8mEgxZmFU/D0MViBw/suAF8f5IopDTN3SoJPI6Ow== 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-SenderADCheck; bh=NIOZAjTZS093pQIewqwtc+bPoMKVJPV9/hsUlTwDMjk=; b=Xsl+ftEoA3759VaK0lXkFpaDZPc68pDkqUanXlGmmp0cab3qU1jmMrp7cgdS/xlIIIvNryBSIw//+eW/J0SDEMupMrwwF8pAnvNWYFeAdkr+H0p+aO50hZUSUaqXSSZHxhWLZf1OirxbBbyucnI4TtZwCgv4wq7q2QXaB1j4oa2NKj4rGT+XaVS5nFxyKtO8KyX3OLJHlGYv8DJW5ltW1UvEekidDtiNe7iB4gg0GdCBcMQsvEpuFEJiI2AFOPpYFvr8jTBMJ0n0qOnJ0LZoDWgvqAR7hpDgRmuXpAP+ze69TAKkZ4TOhvDF1H1qSUq3XuZahbpeXgcu8jnWKpaCWw== 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 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=NIOZAjTZS093pQIewqwtc+bPoMKVJPV9/hsUlTwDMjk=; b=ZcPVbd6Ps9SC4jx/DNEVpiTwVnVRqBWYi2BcTHnRoLSqsgiXj4Pw74YFgvSkjVt3QtQuPh1yFI+k2H36H9jy2EbB2lryhE4GIvg//TKbJLYCxYtW96jboQjp9vZHLKAFz44ZmuxFn7o9XvC8F24RS0ZekWf6UXIvCVdtHc0Rfu0= Received: from DM5PR1101MB2347.namprd11.prod.outlook.com (2603:10b6:3:a2::7) by DM6PR11MB3836.namprd11.prod.outlook.com (2603:10b6:5:13a::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3326.23; Wed, 2 Sep 2020 01:55:35 +0000 Received: from DM5PR1101MB2347.namprd11.prod.outlook.com ([fe80::d167:874f:daa9:9bc2]) by DM5PR1101MB2347.namprd11.prod.outlook.com ([fe80::d167:874f:daa9:9bc2%3]) with mapi id 15.20.3326.025; Wed, 2 Sep 2020 01:55:35 +0000 From: "Xu, Min M" To: Laszlo Ersek , edk2-devel-groups-io CC: "Wang, Jian J" , "Yao, Jiewen" , Wenyi Xie Subject: Re: [PATCH 1/3] SecurityPkg/DxeImageVerificationLib: extract SecDataDirEnd, SecDataDirLeft Thread-Topic: [PATCH 1/3] SecurityPkg/DxeImageVerificationLib: extract SecDataDirEnd, SecDataDirLeft Thread-Index: AQHWgEAPZBMF43Oy2U+15gwMtzNvSqlUlwKg Date: Wed, 2 Sep 2020 01:55:35 +0000 Message-ID: References: <20200901091221.20948-1-lersek@redhat.com> <20200901091221.20948-2-lersek@redhat.com> In-Reply-To: <20200901091221.20948-2-lersek@redhat.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMWMwNmQ4OTktMGFiNy00OGM1LWFmZTgtOGY0NDIzZTE3MzliIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiTjhsRjRtcVdiVVV0RjdHMmliTHc0ZFBIa2NrZDNOQ3c4dWJJa1pQQVJhQ1pBZERtQlM3NXNFb1VGKytxcko1TiJ9 dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 x-ctpclassification: CTP_NT authentication-results: redhat.com; dkim=none (message not signed) header.d=none;redhat.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.147.213] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 6ca695a3-b94d-4fd4-c613-08d84ee34866 x-ms-traffictypediagnostic: DM6PR11MB3836: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:6790; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: j0HSJZM7JxCfnbIXZaPOYOW7Lnule/c5K9J9+7tJGnUEYAXlSQ7Ax38wmGbYqWaWTmp5hJJpqH6Ec+rXVHg/hi/qRY62Rsydlap8EXPRfHUysfeUziXMYgfnZ8rWcmf0PbD3QwzKMWS/Dfq3AWLfxQ9ddac6sQa1SoAS9tDmL49vx4Gy5DT2zB4nmp5F1BYx4Wgormebxa/IEXSUk5IcCXgw7dF3GKiMMRDV06Wy/gPva4D0Qm+ybjBKG+pczIUrLrpfM/errXBDJCUNj7R0s5u3hTg0FQrbh4+l2IL7dnuCGMieo7s3ZUk/e6ZYQ9Ajg4W36n2pLQFOIcsQy1HaTeKWqC8FRW1reC9scRJQ+3AYxIO98lmjsfUrkZSbVyJlk+tSp6BcK0zt3OeyeKVzbA== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM5PR1101MB2347.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(396003)(366004)(346002)(376002)(136003)(39860400002)(5660300002)(478600001)(55016002)(966005)(8676002)(15650500001)(33656002)(2906002)(71200400001)(83380400001)(8936002)(52536014)(76116006)(6506007)(7696005)(66556008)(4326008)(53546011)(66946007)(64756008)(316002)(186003)(66476007)(9686003)(110136005)(26005)(54906003)(66446008)(86362001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: BfW4vc1ZTnbIm2TAw11rr1An5TwTOWWm3qSfM0Te9tuFddtkoz05qCCgAby3hGVXUbn2GlLlKzTyRu0dEyVBFwqwx5xPgANRBUbuW3p0+O1BdFA6K5Ndk9F75ZC124pzFCe7PqO5C08bgPWYRLZiRa8nr9FxaVr9h2XurXPVMuGvZ9Gasd00SCVicG7c24eDKSnkdvO4owlroDa1ArMpaHDnW66AaEZj4+2JqcYpignc1cJ6+5tfu141vN2XfhTO5WrHzNXZ3ZjNLpcgz5xYYPYR6/RUlKy95MXPx3i5xDv48mMwMjKQypPIEGlepvt21GVvXzCgTE1UpRyRQz9caqd6byCErtu+5U3a5m5DAJY8a9uzScua1Le/qRWRkh0S7rXGQJthIv6REW5sSGxaxQSKbLNO3UUymmEztemqbYaYzom0BlllR0gR/DW19qQm2qOxpQ5oUpetPyW28cjqbzAH9kSmV6GIHxw45u9XVcQkItzd7CdsA+Q/LhD7Ph1i1MLhXgUzSABEMh90WLyKqWBXCwzj8qO4ReKXcwG5Yfd/Aiu/+nZyunYI0BaioSxrvWWMfIUVqcXXhRRlF0IAKQJlefvH0KiiJmg/peheUUrb7Mc3vuvH5eJpQi3EWuYNW2awoLAGMLDzhFhLKmvdiw== MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM5PR1101MB2347.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6ca695a3-b94d-4fd4-c613-08d84ee34866 X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Sep 2020 01:55:35.0733 (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: yeKZrGkSiNalcLqdGX5VvC94e8yNOeCba0AJ/oa8LyngnCBL5vfy+TnlON/d3ROT1xkuDajWMKokukZ1vmRPVg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB3836 Return-Path: min.m.xu@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Laszlo Ersek > Sent: Tuesday, September 01, 2020 5:12 PM > To: edk2-devel-groups-io > Cc: Wang, Jian J ; Yao, Jiewen > ; Xu, Min M ; Wenyi Xie > > Subject: [PATCH 1/3] SecurityPkg/DxeImageVerificationLib: extract > SecDataDirEnd, SecDataDirLeft >=20 > The following two quantities: >=20 > SecDataDir->VirtualAddress + SecDataDir->Size > SecDataDir->VirtualAddress + SecDataDir->Size - OffSet >=20 > are used multiple times in DxeImageVerificationHandler(). Introduce helpe= r > variables for them: "SecDataDirEnd" and "SecDataDirLeft", respectively. > This saves us multiple calculations and significantly simplifies the code= . >=20 > Note that all three summands above have type UINT32, therefore the new > variables are also of type UINT32. >=20 > This patch does not change behavior. >=20 > (Note that the code already handles the case when the >=20 > SecDataDir->VirtualAddress + SecDataDir->Size >=20 > UINT32 addition overflows -- namely, in that case, the certificate loop i= s > never entered, and the corruption check right after the loop fires.) >=20 > Cc: Jian J Wang > Cc: Jiewen Yao > Cc: Min Xu > Cc: Wenyi Xie > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2215 > Signed-off-by: Laszlo Ersek Reviewed-by: Min M Xu > --- > SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | = 12 > ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) >=20 > diff --git > a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c > b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c > index b08fe24e85aa..377feebb205a 100644 > --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib= .c > +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLi > +++ b.c > @@ -1652,6 +1652,8 @@ DxeImageVerificationHandler ( > UINT8 *AuthData; > UINTN AuthDataSize; > EFI_IMAGE_DATA_DIRECTORY *SecDataDir; > + UINT32 SecDataDirEnd; > + UINT32 SecDataDirLeft; > UINT32 OffSet; > CHAR16 *NameStr; > RETURN_STATUS PeCoffStatus; > @@ -1849,12 +1851,14 @@ DxeImageVerificationHandler ( > // "Attribute Certificate Table". > // The first certificate starts at offset (SecDataDir->VirtualAddress)= from the > start of the file. > // > + SecDataDirEnd =3D SecDataDir->VirtualAddress + SecDataDir->Size; > for (OffSet =3D SecDataDir->VirtualAddress; > - OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size); > + OffSet < SecDataDirEnd; > OffSet +=3D (WinCertificate->dwLength + ALIGN_SIZE (WinCertificat= e- > >dwLength))) { > WinCertificate =3D (WIN_CERTIFICATE *) (mImageBase + OffSet); > - if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <=3D si= zeof > (WIN_CERTIFICATE) || > - (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < > WinCertificate->dwLength) { > + SecDataDirLeft =3D SecDataDirEnd - OffSet; > + if (SecDataDirLeft <=3D sizeof (WIN_CERTIFICATE) || > + SecDataDirLeft < WinCertificate->dwLength) { > break; > } >=20 > @@ -1948,7 +1952,7 @@ DxeImageVerificationHandler ( > } > } >=20 > - if (OffSet !=3D (SecDataDir->VirtualAddress + SecDataDir->Size)) { > + if (OffSet !=3D SecDataDirEnd) { > // > // The Size in Certificate Table or the attribute certificate table = is > corrupted. > // > -- > 2.19.1.3.g30247aa5d201 >=20