From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web11.20112.1677839760320598177 for ; Fri, 03 Mar 2023 02:36:00 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=G/ouBh1s; spf=pass (domain: redhat.com, ip: 170.10.129.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1677839759; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=+FTzenmBHIEXMnfWfZkFsU1JMhIteNLE0si37lSQc4E=; b=G/ouBh1s1pB92WZ10popIxVY8l38NNqrQUCpPE724ICkI6UpGbK2bG0J1OjwgKBqHxS8vb O0VMrBp1/7CFUNoAwhUE3qBK8gJybthcroyvy4Xt2pUI7IkkGHGrAjRPfQKypU1rwAkD9I TF5hOp6hZ0ZQ0ErCjJKtiRBCf4dzFpw= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-659-s-39DQRzMU-42K6-MrbW7g-1; Fri, 03 Mar 2023 05:35:56 -0500 X-MC-Unique: s-39DQRzMU-42K6-MrbW7g-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E05F7181E3FC; Fri, 3 Mar 2023 10:35:55 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.192.43]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4BF32140EBF6; Fri, 3 Mar 2023 10:35:55 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 71052180061F; Fri, 3 Mar 2023 11:35:53 +0100 (CET) From: "Gerd Hoffmann" To: devel@edk2.groups.io Cc: Pawel Polawski , Jian J Wang , Oliver Steffen , Min Xu , =?UTF-8?q?Marvin=20H=C3=A4user?= , Jiewen Yao , jmaloy@redhat.com, Gerd Hoffmann Subject: [PATCH v2 1/1] SecurityPkg/DxeImageVerificationLib: Check result of GetEfiGlobalVariable2 Date: Fri, 3 Mar 2023 11:35:53 +0100 Message-Id: <20230303103553.804781-1-kraxel@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true Call gRT->GetVariable() directly to read the SecureBoot variable. It is one byte in size so we can easily place it on the stack instead of having GetEfiGlobalVariable2() allocate it for us, which avoids a few possible error cases. Skip secure boot checks if (and only if): (a) the SecureBoot variable is not present (EFI_NOT_FOUND) according to the return value, or (b) the SecureBoot variable was read successfully and is set to SECURE_BOOT_MODE_DISABLE. Previously the code skipped the secure boot checks on *any* gRT->GetVariable() error (GetEfiGlobalVariable2 sets the variable value to NULL in that case) and also on memory allocation failures. Fixes: CVE-2019-14560 Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=2167 Signed-off-by: Gerd Hoffmann --- .../DxeImageVerificationLib.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c index 66e2f5eaa3c0..b3d40c21e975 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1671,7 +1671,8 @@ DxeImageVerificationHandler ( EFI_IMAGE_EXECUTION_ACTION Action; WIN_CERTIFICATE *WinCertificate; UINT32 Policy; - UINT8 *SecureBoot; + UINT8 SecureBoot; + UINTN SecureBootSize; PE_COFF_LOADER_IMAGE_CONTEXT ImageContext; UINT32 NumberOfRvaAndSizes; WIN_CERTIFICATE_EFI_PKCS *PkcsCertData; @@ -1686,6 +1687,8 @@ DxeImageVerificationHandler ( RETURN_STATUS PeCoffStatus; EFI_STATUS HashStatus; EFI_STATUS DbStatus; + EFI_STATUS VarStatus; + UINT32 VarAttr; BOOLEAN IsFound; SignatureList = NULL; @@ -1742,24 +1745,26 @@ DxeImageVerificationHandler ( CpuDeadLoop (); } - GetEfiGlobalVariable2 (EFI_SECURE_BOOT_MODE_NAME, (VOID **)&SecureBoot, NULL); + SecureBootSize = sizeof (SecureBoot); + VarStatus = gRT->GetVariable (EFI_SECURE_BOOT_MODE_NAME, &gEfiGlobalVariableGuid, &VarAttr, &SecureBootSize, &SecureBoot); // // Skip verification if SecureBoot variable doesn't exist. // - if (SecureBoot == NULL) { + if (VarStatus == EFI_NOT_FOUND) { return EFI_SUCCESS; } // // Skip verification if SecureBoot is disabled but not AuditMode // - if (*SecureBoot == SECURE_BOOT_MODE_DISABLE) { - FreePool (SecureBoot); + if ((VarStatus == EFI_SUCCESS) && + (VarAttr == (EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS)) && + (SecureBoot == SECURE_BOOT_MODE_DISABLE)) + { return EFI_SUCCESS; } - FreePool (SecureBoot); - // // Read the Dos header. // -- 2.39.2