From nobody Tue May 7 00:28:08 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+95779+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+95779+1787277+3901457@groups.io; arc=fail (BodyHash is different from the expected one); dmarc=fail(p=none dis=none) header.from=intel.com Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1667255563373942.4599302322854; Mon, 31 Oct 2022 15:32:43 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id 7KnGYY1788612xVUTwQF9WuU; Mon, 31 Oct 2022 15:32:42 -0700 X-Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mx.groups.io with SMTP id smtpd.web10.53.1667255560678590778 for ; Mon, 31 Oct 2022 15:32:41 -0700 X-IronPort-AV: E=McAfee;i="6500,9779,10517"; a="288728099" X-IronPort-AV: E=Sophos;i="5.95,229,1661842800"; d="scan'208,217";a="288728099" X-Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Oct 2022 15:32:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10517"; a="722966749" X-IronPort-AV: E=Sophos;i="5.95,229,1661842800"; d="scan'208,217";a="722966749" X-Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by FMSMGA003.fm.intel.com with ESMTP; 31 Oct 2022 15:32:31 -0700 X-Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) by ORSMSX602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Mon, 31 Oct 2022 15:32:31 -0700 X-Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX611.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Mon, 31 Oct 2022 15:32:31 -0700 X-Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31 via Frontend Transport; Mon, 31 Oct 2022 15:32:31 -0700 X-Received: from NAM11-CO1-obe.outbound.protection.outlook.com (104.47.56.170) 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.2375.31; Mon, 31 Oct 2022 15:32:30 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KGtN6s4NevTfN7a9Mma0V2FEkmEQkk3eSJCGaB+nMDKYSTwa+Okg+vFFWKEV6ByCPkUcYk3c05hX/+eIuKhHxse8fxErgwsvaOxFdi+MQ5dOk8/GyRnc94o7Zj/Y2TeJoX/WmXBdh9U7R64vtLGLS9z1EdZKfgSC+0RA8hrqJ6tGmTGiMT+d5ULv3eEcQyhxzPBol2nWy1puMZhzy8AslsWTh2zvA5Mfq6uHsUAcGQ76OBlOywUEMvEkUL7Ee5+8KJ5FyL2B+8vqW+bf8MOJD9icrAovd+8uc/ML5FLHbeJdEjRqrYTVXfOm3Lz7vIQt3w6mWCybyl4ci8PN+TWzZw== 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=OBPS+NzGAm0yFfvVCBfi4Hci5hyZLk2fUlNPOw/3U+Q=; b=jZ/C0SyG1kuNpSMldLvwfI+DX7p1dvGjnshSvo04S00Dqrf2L2uKnbb5ZX6O9WWl9XZ0j6VtSnOMPDC3a7Kx5/LslZ2AIFHJ7y6PW3wA0jMsOy/qTRNQvUriMaKET62P8cgmE/VsbJ45h/R6LjkrjQrKtHBte37s8FAQ+q2Z9lnVuw/gN1qzMygNT0PE2W0wrWpj9wAappi5Z04KtPlLPO+seKcpW1/brWzIg1UgqVy7+3+5tRac5xIX45vLTEV66sPKZ3oGPAZdpTUYWIM9YQ63Jwmr7VhQac9iKNyqEXCTDDCw0NZg/eaHDXgDb73w73kUqaZ+comphKr8E5eRLg== 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 X-Received: from DS0PR11MB6445.namprd11.prod.outlook.com (2603:10b6:8:c6::11) by CH0PR11MB5409.namprd11.prod.outlook.com (2603:10b6:610:d0::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5769.21; Mon, 31 Oct 2022 22:32:26 +0000 X-Received: from DS0PR11MB6445.namprd11.prod.outlook.com ([fe80::65ad:50c9:7c98:1a35]) by DS0PR11MB6445.namprd11.prod.outlook.com ([fe80::65ad:50c9:7c98:1a35%4]) with mapi id 15.20.5769.015; Mon, 31 Oct 2022 22:32:26 +0000 From: "Demeter, Miki" To: "devel@edk2.groups.io" CC: "Kinney, Michael D" , "Gao, Liming" , "Wang, Jian J" Subject: [edk2-devel] [Patch v4] MdeModulePkg/PiSmmCoreSmmEntryPoint underflow(CVE-2021-38578) Thread-Topic: [Patch v4] MdeModulePkg/PiSmmCoreSmmEntryPoint underflow(CVE-2021-38578) Thread-Index: AQHY7XgcfUj9X4Cq3EGvv+abewVBPg== Date: Mon, 31 Oct 2022 22:32:25 +0000 Message-ID: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: DS0PR11MB6445:EE_|CH0PR11MB5409:EE_ x-ms-office365-filtering-correlation-id: 71475acd-149b-48ac-2e8e-08dabb8fc966 x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: bOggn818VgzcmszJnnp6pfsfi617X5HbGzSYDcJSRZqGqALwIvAUznChRgvv84mjXapM+zSfILBp+sfACA/WsRGMTFprKzFB3eOwm7qUJtlzJvKjA0o9Kk+lk8OdrITe1ub2p51w1DbZxu5CyGTqHJjXMPiZBMtU9B4BNKUPnJg6wgHPAmRCmvR4ptKIExgbtsgoy08yz2NwHgMEubXcpDq9HIZEo+QeqgeCVhv9RiQ5QT9srpvqNWK41hveuvXVytZFNm/+hyDMET4Qj8uNZAkL/p3gGkhSjmKj2fQxGFc8CpLxyZcQ271W0bjybIQfzf32h2MLUGrtFNjJPolgWU4rLkz95Xe3QOQZYUz3gH2t0sJRHYrXrdWbLm5ozuVEznOZrdToXexZAeqiwVmSKjmEeMWJL2JfLl4UzwgS3MC9/TqF07rpKjE7GWnT6sn0QnUUJtdENARnCmSFGMDZR5ReqS1uinAr9VfuKaIdKvIPXrV1Idj0WHtmm0pM3v4SOcH2pFFGs91w/Px6ANYGqKLu4u7kJVzEZCLjp7+UFncI4AQn/yxIhg6C1W9K4tjQU1kJuxomJl0OpPXxBqmn6CD2ogOT8QhdoYzJi+4NuWOYZ5Um91hlVhqUl4IyLQAz/E4uLk9ZIm5x5XSfBxXH+hzixZwu7aWvUcU3ykTSWP08VGc7vxps7CWGtBYKnccn+dXlwvBvbOHME4L/3JE9wFJZQSusl1QKdgg0suIIYEQ1EDErabzZUy4jRIw7DGAXTI6II7Gy/30qeuxxq4/TojtnHx3Do6AE9c2jyr6TrJhIYx+zBnLwEfDiRr6CnqkusJPvoj1/LUj6LAm0X5JVnQ== x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?WpBcxGJfxFurojMOhJsrx8RnkpwfxY5MFG76/rxfFaiTT/LDc4bDAnM4tJ23?= =?us-ascii?Q?9JM3/DlAE+UNCUvbQLUPJ1/w6TP/WmspOgOhX7y6cb1IBu5aWJfJTWjSJgJ4?= =?us-ascii?Q?m3Q1pJM3vqsmeRqDeW1HL5DXo+1wU8W25fACdo2r+CTXTD47ejar3zTYGEzo?= =?us-ascii?Q?kLzgbtH4GXYmz7HlkHZkrCbYI5VyFC+l4nkXY6RRsPOCLlEBGkpw4zrcvS+E?= =?us-ascii?Q?/v9w5GOUVRWvZ1cCQ3BhhZ5tCnxPX9KxVYrn32Q11T5HWJ+rAsP5WZnIXjWi?= =?us-ascii?Q?9sBh4ybM99HGY9PNjfDM1paL2jTIWdNyefkekXmhFJbo+rSRDSGHfRUFhxKL?= =?us-ascii?Q?1VhcwkOxiqWJyaJeUUmB0t19S6QtU5Gk//DQPE2A5YJS42U0uOQu6USRnQiH?= =?us-ascii?Q?eOvI6bZL2guJkpS5I4eOGN+tO1zlbPq0E2hNZ/sTfUXR4//TshYHQ1hwhEel?= =?us-ascii?Q?df5bdTbv+ap+coCtfIUZRNTsVZNw4ZbHnMgZAlxPoJigivjQ8fAB/KTnkGSB?= =?us-ascii?Q?M31rpcrqH70JLy8u+TeEFnF5Y5HPrVtANez8at2bYKj7CP2q4oeG5ZvlcCCK?= =?us-ascii?Q?to9yngFnnmNMx6SAqG18rz0K2496kYkpUijXodqeTVDDl76Nt/7TClYGUUlK?= =?us-ascii?Q?oIojmcciKhIfrPKJLdmlNTABesaISiZjiA/UoQfcxnyU4ndQJzgoVr9tVg4c?= =?us-ascii?Q?hA86DSZ9xB2wEVTcGOfuL5YYH9qzRkTzm+P0wttEKT6xhKf86weenrQRgIjn?= =?us-ascii?Q?8QkJsHc3eHnOXFyH8aM4cStUu+4rMi7ui9QrDEtQbLbf3JOnRck/jPo/ZWIQ?= =?us-ascii?Q?WmEn3ZjOFjeuQA2c29vcngNKZ+6UuLuA7LzkmzcFfEyOC5B60O5xdVWOB5Qd?= =?us-ascii?Q?5isJTt1UnStVw1o1RIS7suAyqo1WTaZHqnFgwslm7ugrr0W/DvwsDJ6re4bI?= =?us-ascii?Q?8cJ8aLl6INzyrDmx5ZoeBZRXT4jFT3tS4c7Ep/nfRVjEBzmvng8sTi/j37qP?= =?us-ascii?Q?g9p6epLpkrtwfXnBK3Vlv5yJcHtz9Pmq2TA+zxmO5dZWQXZgVx5VT5sBVAZV?= =?us-ascii?Q?pJcJ4cleedpZc1ulafCaJ+9NiF62uCWLmE9ZdmFck+XfeK8bTbNvFkNwPe13?= =?us-ascii?Q?OozO4bLyqn2oPaBT+S/cfGMtYWLZH6Deo9yMBYGIMLqf9rqtyC0ux3BMleEG?= =?us-ascii?Q?dDTRjkCz2VQEX7ttjJLsUr4Af6+gfwreIjGgsZXVJ27a+RYMJLouG+27mmZH?= =?us-ascii?Q?pk7MaJB6Z2+rb2x5UQgBvz2M6SxMsNvZ4btsDJZW1bOpm42h49vt3pzJ1Cg2?= =?us-ascii?Q?rZ+DCsGZlzQMOH1hHITdrIm+z8XAO4CijZTY7/ECkvPa4nKBaYSIBHVntfdj?= =?us-ascii?Q?lF0864/ar2rt5SByDfPIlmYZ54w/GCKcakeMGGHKP9vUZc3Xh3Q4tM6Uu1fq?= =?us-ascii?Q?dKuMSAOFErZ/hJNgBKmejRk8QfzecczrRQKRAvnqeNP/Dl1sF88vbxXDkKMp?= =?us-ascii?Q?jx3lDTZ38AZQadtL1laxXsSl+b14FlUa+z5JbOVyNOkl+F66PJeLiEwSxFGn?= =?us-ascii?Q?EBdr4SadYoFB4aUiaJBmWxx146hJCln6SBi7/kcbq4SzsS+1+qVhNi0xq+Y2?= =?us-ascii?Q?Zg=3D=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB6445.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 71475acd-149b-48ac-2e8e-08dabb8fc966 X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Oct 2022 22:32:25.9356 (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: F4lcTAv5WJRe5dd3hdtY4CWjBhiCxHYL1OYBR/C5i0cWuOtSeqfPfmmeIHEyrtJM4n6NpsSCqutn460hfUYVOQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR11MB5409 X-OriginatorOrg: intel.com Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,miki.demeter@intel.com X-Gm-Message-State: lUz2sfx07xa4SdwyjWBJwPxUx1787277AA= Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_DS0PR11MB6445C4CCBF58221FFFADD41A8D379DS0PR11MB6445namp_" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1667255562; bh=nVgxpfRarvUCR5EaYxw1jIeQyJjyhEbSNWsW1sWNjNU=; h=CC:Content-Type:Date:From:Reply-To:Subject:To; b=No9Qr18rJYMYW7vFYiriem/94U5rSaPlHY3dWS+IHgiVwt0SmXN0c+iuS/iLlnG0peJ z7YfxiEOX46oooBNXbnpC5SKBUwv+XwFCJ1GttC6eKcFfDc6S/7cZl2A7R8H7Az0PvM9Q b+SDvYZNxmrxZ+hH/nF25c3n664qsRZxvQU= X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1667255564549100001 --_000_DS0PR11MB6445C4CCBF58221FFFADD41A8D379DS0PR11MB6445namp_ Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D3387 Added use of SafeIntLib to validate values are not causing overflows or underflows in user controlled values when calculating buffer sizes. Signed-off-by: Miki Demeter Reviewed-by: Michael D Kinney Cc: Jian J Wang Cc: Liming Gao --- MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 41 ++++++++++++++++++----- MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 1 + MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 1 + MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 31 +++++++++++++---- MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 1 + 5 files changed, 60 insertions(+), 15 deletions(-) diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/Pi= SmmCore/PiSmmCore.c index 9e5c6cbe33..875c7c0258 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c @@ -610,6 +610,7 @@ SmmEndOfS3ResumeHandler ( @param[in] Size2 Size of Buff2 @retval TRUE Buffers overlap in memory. + @retval TRUE Math error. Prevents potential math over and under= flows. @retval FALSE Buffer doesn't overlap. **/ @@ -621,11 +622,24 @@ InternalIsBufferOverlapped ( IN UINTN Size2 ) { + UINTN End1; + UINTN End2; + BOOLEAN IsOverUnderflow1; + BOOLEAN IsOverUnderflow2; + + // Check for over or underflow + IsOverUnderflow1 =3D EFI_ERROR (SafeUintnAdd ((UINTN)Buff1, Size1, &End1= )); + IsOverUnderflow2 =3D EFI_ERROR (SafeUintnAdd ((UINTN)Buff2, Size2, &End2= )); + + if (IsOverUnderflow1 || IsOverUnderflow2) { + return TRUE; + } + // // If buff1's end is less than the start of buff2, then it's ok. // Also, if buff1's start is beyond buff2's end, then it's ok. // - if (((Buff1 + Size1) <=3D Buff2) || (Buff1 >=3D (Buff2 + Size2))) { + if ((End1 <=3D (UINTN)Buff2) || ((UINTN)Buff1 >=3D End2)) { return FALSE; } @@ -651,6 +665,7 @@ SmmEntryPoint ( EFI_SMM_COMMUNICATE_HEADER *CommunicateHeader; BOOLEAN InLegacyBoot; BOOLEAN IsOverlapped; + BOOLEAN IsOverUnderflow; VOID *CommunicationBuffer; UINTN BufferSize; @@ -699,23 +714,31 @@ SmmEntryPoint ( (UINT8 *)gSmmCorePrivate, sizeof (*gSmmCorePrivate) ); - if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferS= ize) || IsOverlapped) { + // + // Check for over or underflows + // + IsOverUnderflow =3D EFI_ERROR (SafeUintnSub (BufferSize, OFFSET_OF (= EFI_SMM_COMMUNICATE_HEADER, Data), &BufferSize)); + + if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferS= ize) || + IsOverlapped || IsOverUnderflow) + { // // If CommunicationBuffer is not in valid address scope, // or there is overlap between gSmmCorePrivate and CommunicationBu= ffer, + // or there is over or underflow, // return EFI_INVALID_PARAMETER // gSmmCorePrivate->CommunicationBuffer =3D NULL; gSmmCorePrivate->ReturnStatus =3D EFI_ACCESS_DENIED; } else { CommunicateHeader =3D (EFI_SMM_COMMUNICATE_HEADER *)CommunicationB= uffer; - BufferSize -=3D OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data); - Status =3D SmiManage ( - &CommunicateHeader->HeaderGuid, - NULL, - CommunicateHeader->Data, - &BufferSize - ); + // BufferSize was updated by the SafeUintnSub() call above. + Status =3D SmiManage ( + &CommunicateHeader->HeaderGuid, + NULL, + CommunicateHeader->Data, + &BufferSize + ); // // Update CommunicationBuffer, BufferSize and ReturnStatus // Communicate service finished, reset the pointer to CommBuffer t= o NULL diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/Pi= SmmCore/PiSmmCore.h index 71422b9dfc..b8a490a8c3 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h @@ -54,6 +54,7 @@ #include #include #include +#include #include "PiSmmCorePrivateData.h" #include "HeapGuard.h" diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf b/MdeModulePkg/Core/= PiSmmCore/PiSmmCore.inf index c8bfae3860..3df44b38f1 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf @@ -60,6 +60,7 @@ PerformanceLib HobLib SmmMemLib + SafeIntLib [Protocols] gEfiDxeSmmReadyToLockProtocolGuid ## UNDEFINED # SmiHandlerR= egister diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiS= mmCore/PiSmmIpl.c index 4f00cebaf5..fbba868fd0 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c @@ -34,8 +34,8 @@ #include #include #include - #include "PiSmmCorePrivateData.h" +#include #define SMRAM_CAPABILITIES (EFI_MEMORY_WB | EFI_MEMORY_UC) @@ -1354,6 +1354,7 @@ SmmSplitSmramEntry ( @param[in] ReservedRangeToCompare Pointer to EFI_SMM_RESERVED_SMRAM_= REGION to compare. @retval TRUE There is overlap. + @retval TRUE Math error. @retval FALSE There is no overlap. **/ @@ -1363,11 +1364,29 @@ SmmIsSmramOverlap ( IN EFI_SMM_RESERVED_SMRAM_REGION *ReservedRangeToCompare ) { - UINT64 RangeToCompareEnd; - UINT64 ReservedRangeToCompareEnd; - - RangeToCompareEnd =3D RangeToCompare->CpuStart + RangeToCompare-= >PhysicalSize; - ReservedRangeToCompareEnd =3D ReservedRangeToCompare->SmramReservedStart= + ReservedRangeToCompare->SmramReservedSize; + UINT64 RangeToCompareEnd; + UINT64 ReservedRangeToCompareEnd; + BOOLEAN IsOverUnderflow1; + BOOLEAN IsOverUnderflow2; + + // Check for over or underflow. + IsOverUnderflow1 =3D EFI_ERROR ( + SafeUint64Add ( + (UINT64)RangeToCompare->CpuStart, + RangeToCompare->PhysicalSize, + &RangeToCompareEnd + ) + ); + IsOverUnderflow2 =3D EFI_ERROR ( + SafeUint64Add ( + (UINT64)ReservedRangeToCompare->SmramReservedStar= t, + ReservedRangeToCompare->SmramReservedSize, + &ReservedRangeToCompareEnd + ) + ); + if (IsOverUnderflow1 || IsOverUnderflow2) { + return TRUE; + } if ((RangeToCompare->CpuStart >=3D ReservedRangeToCompare->SmramReserved= Start) && (RangeToCompare->CpuStart < ReservedRangeToCompareEnd)) diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf b/MdeModulePkg/Core/P= iSmmCore/PiSmmIpl.inf index 6109d6b544..ddeb39cee2 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf @@ -46,6 +46,7 @@ DxeServicesLib PcdLib ReportStatusCodeLib + SafeIntLib [Protocols] gEfiSmmBase2ProtocolGuid ## PRODUCES -- 2.21.0 -- -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#95779): https://edk2.groups.io/g/devel/message/95779 Mute This Topic: https://groups.io/mt/94697674/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --_000_DS0PR11MB6445C4CCBF58221FFFADD41A8D379DS0PR11MB6445namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

REF:https://bugzilla.tianocore.org/show_= bug.cgi?id=3D3387

 

Added use of SafeIntLib to validate valu= es are not causing overflows or

underflows in user controlled values whe= n calculating buffer sizes.

 

Signed-off-by: Miki Demeter <miki.dem= eter@intel.com>

Reviewed-by: Michael D Kinney <michae= l.d.kinney@intel.com>

Cc: Jian J Wang <jian.j.wang@intel.co= m>

Cc: Liming Gao <gaoliming@byosoft.com= .cn>

---

 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c   | 41 ++++++++++++++++++-----

 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h   |  1 +

 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf |  1 +

 MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c    | 31 +++++++++++++----

 MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf  |&nb= sp; 1 +

 5 files changed, 60 insertions(+), 15 deletions(-)

 

diff --git a/MdeModulePkg/Core/PiSmmCore= /PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c

index 9e5c6cbe33..875c7c0258 100644

--- a/MdeModulePkg/Core/PiSmmCore/PiSmmC= ore.c

+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmC= ore.c

@@ -610,6 +610,7 @@ SmmEndOfS3ResumeHand= ler (

   <= span class=3D"s1">@param[in] Size2  Size of Buff2

 

   <= span class=3D"s1">@retval TRUE=       Buffers overlap in memory.

+  @retval TRUE      Math error.     Prevents potential math over and underflows.=

   <= span class=3D"s1">@retval FALSE     Buffer doesn't overlap.

 

 **/

@@ -621,11 +622,24 @@ InternalIsBufferOv= erlapped (

   <= span class=3D"s1">IN UINTN&nbs= p; Size2

   <= span class=3D"s1">)

 {

+  UINTN =   End1;

+  UINTN =   End2;

+  BOOLEAN&nbs= p; IsOverUnderflow1;

+  BOOLEAN&nbs= p; IsOverUnderflow2;

+

+  // Check for over or underflow

+  IsOverUnderflow1 =3D EFI_ERROR (SafeUintnAdd ((UINTN)Buf= f1, Size1, &End1));

+  IsOverUnderflow2 =3D EFI_ERROR (SafeUintnAdd ((UINTN)Buf= f2, Size2, &End2));

+

+  if (IsOverUnderflow1 || IsOverUnderflow2) {<= /o:p>

+    return TRUE;

+  }

+

   <= span class=3D"s1">//

   <= span class=3D"s1">// If buff1's end is less than the start of buff2, then i= t's ok.

   <= span class=3D"s1">// Also, if buff1's start is beyond buff2's end, then it'= s ok.

   <= span class=3D"s1">//

-  if (((Buff1 + Size1) <=3D Buff2) || (Buff1 >=3D (B= uff2 + Size2))) {

+  if ((End1 <=3D (UINTN)Buff2) || ((UINTN)Buff1 >=3D= End2)) {

     <= /span>return FALSE;

   <= span class=3D"s1">}

 

@@ -651,6 +665,7 @@ SmmEntryPoint (

   <= span class=3D"s1">EFI_SMM_COMMUNICATE_HEADER  *CommunicateHeader;

   <= span class=3D"s1">BOOLEAN         &n= bsp;           InLegacyB= oot;

   <= span class=3D"s1">BOOLEAN         &n= bsp;           IsOverlap= ped;

+  BOOLEAN &nb= sp;                   IsOverUnderflow;

   <= span class=3D"s1">VOID  &= nbsp;                     *CommunicationBuffer;

   <= span class=3D"s1">UINTN         &n= bsp;             Bu= fferSize;

 

@@ -699,23 +714,31 @@ SmmEntryPoint (

      =                   (UINT8 *)gSmmCorePrivate,

      =                   sizeof (*gSmmCorePrivate)

      =                   );

-      if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuf= fer, BufferSize) || IsOverlapped) {

+      //

+      // Check for over or underflows

+      //

+      IsOverUnderflow =3D EFI_ERROR (SafeUintnSub (BufferSize,= OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data), &BufferSize));

+

+      if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuf= fer, BufferSize) ||

+          IsOverlapped || IsOverUnderflow)

+      {

     &= nbsp;   //

     &= nbsp;   // If CommunicationBuffer is not in = valid address scope,

     &= nbsp;   // or there is overlap between gSmmC= orePrivate and CommunicationBuffer,

+        // or there is over or underflow,

     &= nbsp;   // return EFI_INVALID_PARAMETER

     &= nbsp;   //

     &= nbsp;   gSmmCorePrivate->CommunicationBuf= fer =3D NULL;

     &= nbsp;   gSmmCorePrivate->ReturnStatus        =3D EFI_ACCESS_DENIED;

     &= nbsp; } else {

     &= nbsp;   CommunicateHeader =3D (EFI_SMM_COMMU= NICATE_HEADER *)CommunicationBuffer;

-        BufferSize       -=3D OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data)= ;

-        Status            =3D SmiManage (

-                   =           &CommunicateHeader->HeaderGuid,

-                   =           NULL,

-                   =           CommunicateHeader->Data,

-                   =           &BufferSize

-                   =           );

+        // BufferSize was updated by the SafeUintnSub() c= all above.

+        Status =3D SmiManage (

+                   &CommunicateHeader->HeaderGuid,

+                   NULL,

+                   CommunicateHeader->Data,

+                   &BufferSize

+                   );

     &= nbsp;   //

     &= nbsp;   // Update CommunicationBuffer, Buffe= rSize and ReturnStatus

     &= nbsp;   // Communicate service finished, res= et the pointer to CommBuffer to NULL

diff --git a/MdeModulePkg/Core/PiSmmCore= /PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h

index 71422b9dfc..b8a490a8c3 100644

--- a/MdeModulePkg/Core/PiSmmCore/PiSmmC= ore.h

+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmC= ore.h

@@ -54,6 +54,7 @@

 #include <Library/PerformanceLib.h>

 #include <Library/HobLib.h>

 #include <Library/SmmMemLib.h>

+#include <Library/SafeIntLib.h>

 

 #include "PiSmmCorePrivateData.h"

 #include "HeapGuard.h"

diff --git a/MdeModulePkg/Core/PiSmmCore= /PiSmmCore.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf

index c8bfae3860..3df44b38f1 100644

--- a/MdeModulePkg/Core/PiSmmCore/PiSmmC= ore.inf

+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmC= ore.inf

@@ -60,6 +60,7 @@

   <= span class=3D"s1">PerformanceLib

   <= span class=3D"s1">HobLib

   <= span class=3D"s1">SmmMemLib

+  SafeIntLib

 

 [Protocols]

   <= span class=3D"s1">gEfiDxeSmmReadyToLockProtocolGuid         &n= bsp;   ## UNDEFINED # SmiHandlerRegister

diff --git a/MdeModulePkg/Core/PiSmmCore= /PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c

index 4f00cebaf5..fbba868fd0 100644

--- a/MdeModulePkg/Core/PiSmmCore/PiSmmI= pl.c

+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmI= pl.c

@@ -34,8 +34,8 @@

 #include <Library/UefiRuntimeLib.h>

 #include <Library/PcdLib.h>

 #include <Library/ReportStatusCodeLib.h>=

-

 #include "PiSmmCorePrivateData.h"

+#include <Library/SafeIntLib.h>

 

 #define SMRAM_CAPABILITIES  (EFI_MEMORY_WB | EFI_MEMORY_UC)=

 

@@ -1354,6 +1354,7 @@ SmmSplitSmramEntry= (

   <= span class=3D"s1">@param[in] ReservedRangeToCompare     Pointer to EFI_SMM_RESERVED_SMRAM_REGION to compare.<= /o:p>

 

   <= span class=3D"s1">@retval TRUE=   There is overlap.

+  @retval TRUE  Math error.

   <= span class=3D"s1">@retval FALSE There is no overlap.

 

 **/

@@ -1363,11 +1364,29 @@ SmmIsSmramOverla= p (

   <= span class=3D"s1">IN EFI_SMM_RESERVED_SMRAM_REGION  *ReservedRangeToCompare

   <= span class=3D"s1">)

 {

-  UINT64 = ; RangeToCompareEnd;

-  UINT64 = ; ReservedRangeToCompareEnd;

-

-  RangeToCompareEnd         =3D RangeToCompare->CpuStart + RangeToCompare-= >PhysicalSize;

-  ReservedRangeToCompareEnd =3D ReservedRangeToCompare->= ;SmramReservedStart + ReservedRangeToCompare->SmramReservedSize;<= o:p>

+  UINT64 &nbs= p; RangeToCompareEnd;

+  UINT64 &nbs= p; ReservedRangeToCompareEnd;

+  BOOLEAN&nbs= p; IsOverUnderflow1;

+  BOOLEAN&nbs= p; IsOverUnderflow2;

+

+  // Check for over or underflow.

+  IsOverUnderflow1 =3D EFI_ERROR (

+                    = ;   SafeUint64Add (

+                    = ;     (UINT64)RangeToCompare->CpuStart,<= /o:p>

+                    = ;     RangeToCompare->PhysicalSize,

+                    = ;     &RangeToCompareEnd

+                    = ;     )

+                    = ;   );

+  IsOverUnderflow2 =3D EFI_ERROR (

+                    = ;   SafeUint64Add (

+                    = ;     (UINT64)ReservedRangeToCompare->SmramReservedS= tart,

+                    = ;     ReservedRangeToCompare->SmramReservedSize,

+                    = ;     &ReservedRangeToCompareEnd<= /p>

+                    = ;     )

+                    = ;   );

+  if (IsOverUnderflow1 || IsOverUnderflow2) {<= /o:p>

+    return TRUE;

+  }

 

   <= span class=3D"s1">if ((RangeToCompare->CpuStart >=3D ReservedRangeToC= ompare->SmramReservedStart) &&

     &= nbsp; (RangeToCompare->CpuStart < ReservedR= angeToCompareEnd))

diff --git a/MdeModulePkg/Core/PiSmmCore= /PiSmmIpl.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf<= /p>

index 6109d6b544..ddeb39cee2 100644

--- a/MdeModulePkg/Core/PiSmmCore/PiSmmI= pl.inf

+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmI= pl.inf

@@ -46,6 +46,7 @@

   <= span class=3D"s1">DxeServicesLib

   <= span class=3D"s1">PcdLib

   <= span class=3D"s1">ReportStatusCodeLib

+  SafeIntLib

 

 [Protocols]

   <= span class=3D"s1">gEfiSmmBase2ProtocolGuid                  =     ## PRODUCES

-- 

2.21.0

 

 

-- =

 

_._,_._,_
G= roups.io Links:

You receive all messages sent to this group.

Vi= ew/Reply Online (#95779) | | Mute This Topic | New Topic
Your Subscription | Contact Group Owner | Unsubscribe [importer@patchew.org]
_._,_._,_=20 --_000_DS0PR11MB6445C4CCBF58221FFFADD41A8D379DS0PR11MB6445namp_--