From nobody Tue Nov 26 13:52:24 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+53870+1787277+3901457@groups.io; helo=web01.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+53870+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=intel.com Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 1580998780113620.4921556484414; Thu, 6 Feb 2020 06:19:40 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id YnKrYY1788612xrKCwh4EJQY; Thu, 06 Feb 2020 06:19:39 -0800 X-Received: from mga02.intel.com (mga02.intel.com []) by mx.groups.io with SMTP id smtpd.web11.12696.1580998775768725774 for ; Thu, 06 Feb 2020 06:19:38 -0800 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Feb 2020 06:19:38 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,409,1574150400"; d="scan'208";a="226160679" X-Received: from shwdeopensfp777.ccr.corp.intel.com ([10.239.158.78]) by fmsmga008.fm.intel.com with ESMTP; 06 Feb 2020 06:19:37 -0800 From: "Wang, Jian J" To: devel@edk2.groups.io Cc: Jiewen Yao , Chao Zhang Subject: [edk2-devel] [PATCH 4/9] SecurityPkg/DxeImageVerificationLib: avoid bypass in fetching dbx in IsAllowedByDb(CVE-2019-14575) Date: Thu, 6 Feb 2020 22:19:28 +0800 Message-Id: <20200206141933.356-5-jian.j.wang@intel.com> In-Reply-To: <20200206141933.356-1-jian.j.wang@intel.com> References: <20200206141933.356-1-jian.j.wang@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: 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,jian.j.wang@intel.com X-Gm-Message-State: 1DdEaCNK9QJXREVz5X68CzPux1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1580998779; bh=AEJ1vhnMjuxRXW8uLZpv54GtlfSoD6+IWBYfLl10x7U=; h=Cc:Date:From:Reply-To:Subject:To; b=t/Dx4zH2UgGMTy1s2Xxe0x3QMGK6Omp1g1Ti3DJpIE30KNOWFxAecgNerwQabTTN+2q YLz3mFPxemR0SBNwF8G6PqOT3ePnkmc1TPdOi1cq7mm3GVTvHlYvseU6upLS5YDyaT0tO 3ksFekckg9G9aFH86b75d+JtdtHVl7Rkruo= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1608 In timestamp check after the cert is found in db, the original code jumps to 'Done' if any error happens in fetching dbx variable. At any of the jump, VerifyStatus equals to TRUE, which means allowed-by-db. This should not be allowed except to EFI_NOT_FOUND case (meaning dbx doesn't exist), because it could be used to bypass timestamp check. This patch add code to change VerifyStatus to FALSE in the case of memory allocation failure and dbx fetching failure to avoid potential bypass issue. Cc: Jiewen Yao Cc: Chao Zhang Signed-off-by: Jian J Wang Reviewed-by: Jiewen Yao --- .../DxeImageVerificationLib/DxeImageVerificationLib.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.c index 1efb2f96cd..ed5dbf26b0 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1459,15 +1459,26 @@ IsAllowedByDb ( DbxDataSize =3D 0; Status =3D gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &= gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL); if (Status !=3D EFI_BUFFER_TOO_SMALL) { + if (Status !=3D EFI_NOT_FOUND) { + VerifyStatus =3D FALSE; + } goto Done; } DbxData =3D (UINT8 *) AllocateZeroPool (DbxDataSize); if (DbxData =3D=3D NULL) { + // + // Force not-allowed-by-db to avoid bypass + // + VerifyStatus =3D FALSE; goto Done; } =20 Status =3D gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gE= fiImageSecurityDatabaseGuid, NULL, &DbxDataSize, (VOID *) DbxData); if (EFI_ERROR (Status)) { + // + // Force not-allowed-by-db to avoid bypass + // + VerifyStatus =3D FALSE; goto Done; } =20 --=20 2.24.0.windows.2 -=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 (#53870): https://edk2.groups.io/g/devel/message/53870 Mute This Topic: https://groups.io/mt/71023421/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-