From: "Laszlo Ersek" <lersek@redhat.com>
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: "Dandan Bi" <dandan.bi@intel.com>,
"Hao A Wu" <hao.a.wu@intel.com>,
"Jian J Wang" <jian.j.wang@intel.com>,
"Liming Gao" <gaoliming@byosoft.com.cn>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: [PATCH v2 RESEND 2/2] MdeModulePkg/Core/Dxe: limit FwVol encapsulation section recursion
Date: Thu, 19 Nov 2020 11:53:40 +0100 [thread overview]
Message-ID: <20201119105340.16225-3-lersek@redhat.com> (raw)
In-Reply-To: <20201119105340.16225-1-lersek@redhat.com>
The DXE Core sets up a protocol notify function in its entry point, for
instances of the Firmware Volume Block2 Protocol:
DxeMain() [DxeMain/DxeMain.c]
FwVolDriverInit() [FwVol/FwVol.c]
Assume that a 3rd party UEFI driver or application installs an FVB
instance, with crafted contents. The notification function runs:
NotifyFwVolBlock() [FwVol/FwVol.c]
installing an instance of the Firmware Volume 2 Protocol on the handle.
(Alternatively, assume that a 3rd party application calls
gDS->ProcessFirmwareVolume(), which may also produce a Firmware Volume 2
Protocol instance.)
The EFI_FIRMWARE_VOLUME2_PROTOCOL.ReadSection() member performs "a
depth-first, left-to-right search algorithm through all sections found in
the specified file" (quoting the PI spec), as follows:
FvReadFileSection() [FwVol/FwVolRead.c]
GetSection() [SectionExtraction/CoreSectionExtraction.c]
FindChildNode() [SectionExtraction/CoreSectionExtraction.c]
FindChildNode() // recursive call
FindChildNode() is called recursively for encapsulation sections.
Currently this recursion is not limited. Introduce a new PCD
(fixed-at-build, or patchable-in-module), and make FindChildNode() track
the section nesting depth against that PCD.
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1743
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
MdeModulePkg/MdeModulePkg.dec | 6 ++++
MdeModulePkg/MdeModulePkg.uni | 6 ++++
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
| 33 ++++++++++++++++++--
4 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 00075528198d..9b52b3449443 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1529,6 +1529,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
# @Prompt Enable Capsule On Disk support.
gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleOnDiskSupport|FALSE|BOOLEAN|0x0000002d
+ ## Maximum permitted encapsulation levels of sections in a firmware volume,
+ # in the DXE phase. Minimum value is 1. Sections nested more deeply are
+ # rejected.
+ # @Prompt Maximum permitted FwVol section nesting depth (exclusive).
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth|0x10|UINT32|0x00000030
+
[PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
## This PCD defines the Console output row. The default value is 25 according to UEFI spec.
# This PCD could be set to 0 then console output would be at max column and max row.
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 40884c57a460..1b347a75f684 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -1160,6 +1160,12 @@
"Note:<BR>"
"If Both Capsule In Ram and Capsule On Disk are provisioned at the same time, the Capsule On Disk will be bypassed."
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdFwVolDxeMaxEncapsulationDepth_PROMPT #language en-US "Maximum permitted FwVol section nesting depth (exclusive)."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdFwVolDxeMaxEncapsulationDepth_HELP #language en-US "Maximum permitted encapsulation levels of sections in a firmware volume,<BR>"
+ "in the DXE phase. Minimum value is 1. Sections nested more deeply are<BR>"
+ "rejected."
+
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdCapsuleInRamSupport_PROMPT #language en-US "Enable Capsule In Ram support"
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdCapsuleInRamSupport_HELP #language en-US "Capsule In Ram is to use memory to deliver the capsules that will be processed after system reset.<BR><BR>"
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 1d4b11dc7318..e4bca895773d 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -185,6 +185,7 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth ## CONSUMES
# [Hob]
# RESOURCE_DESCRIPTOR ## CONSUMES
--git a/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c b/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c
index d7f7ef427422..908617d1ca5c 100644
--- a/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c
+++ b/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c
@@ -955,6 +955,9 @@ CreateChildNode (
This is an in/out parameter and it is 1-based,
to deal with recursions.
@param SectionDefinitionGuid Guid of section definition
+ @param Depth Nesting depth of encapsulation sections.
+ Callers different from FindChildNode() are
+ responsible for passing in a zero Depth.
@param FoundChild Output indicating the child node that is found.
@param FoundStream Output indicating which section stream the child
was found in. If this stream was generated as a
@@ -968,6 +971,9 @@ CreateChildNode (
@retval EFI_NOT_FOUND Requested child node does not exist.
@retval EFI_PROTOCOL_ERROR a required GUIDED section extraction protocol
does not exist
+ @retval EFI_ABORTED Recursion aborted because Depth has been
+ greater than or equal to
+ PcdFwVolDxeMaxEncapsulationDepth.
**/
EFI_STATUS
@@ -976,6 +982,7 @@ FindChildNode (
IN EFI_SECTION_TYPE SearchType,
IN OUT UINTN *SectionInstance,
IN EFI_GUID *SectionDefinitionGuid,
+ IN UINT32 Depth,
OUT CORE_SECTION_CHILD_NODE **FoundChild,
OUT CORE_SECTION_STREAM_NODE **FoundStream,
OUT UINT32 *AuthenticationStatus
@@ -990,6 +997,10 @@ FindChildNode (
ASSERT (*SectionInstance > 0);
+ if (Depth >= PcdGet32 (PcdFwVolDxeMaxEncapsulationDepth)) {
+ return EFI_ABORTED;
+ }
+
CurrentChildNode = NULL;
ErrorStatus = EFI_NOT_FOUND;
@@ -1053,6 +1064,7 @@ FindChildNode (
SearchType,
SectionInstance,
SectionDefinitionGuid,
+ Depth + 1,
&RecursedChildNode,
&RecursedFoundStream,
AuthenticationStatus
@@ -1067,9 +1079,17 @@ FindChildNode (
*FoundStream = RecursedFoundStream;
return EFI_SUCCESS;
} else {
+ if (Status == EFI_ABORTED) {
+ //
+ // If the recursive call was aborted due to nesting depth, stop
+ // looking for the requested child node. The skipped subtree could
+ // throw off the instance counting.
+ //
+ return Status;
+ }
//
- // If the status is not EFI_SUCCESS, just save the error code and
- // continue to find the request child node in the rest stream.
+ // Save the error code and continue to find the requested child node in
+ // the rest of the stream.
//
ErrorStatus = Status;
}
@@ -1272,11 +1292,20 @@ GetSection (
*SectionType,
&Instance,
SectionDefinitionGuid,
+ 0, // encapsulation depth
&ChildNode,
&ChildStreamNode,
&ExtractedAuthenticationStatus
);
if (EFI_ERROR (Status)) {
+ if (Status == EFI_ABORTED) {
+ DEBUG ((DEBUG_ERROR, "%a: recursion aborted due to nesting depth\n",
+ __FUNCTION__));
+ //
+ // Map "aborted" to "not found".
+ //
+ Status = EFI_NOT_FOUND;
+ }
goto GetSection_Done;
}
--
2.19.1.3.g30247aa5d201
next prev parent reply other threads:[~2020-11-19 10:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-19 10:53 [PATCH v2 RESEND 0/2] security fix: unlimited FV recursion, round 2 (DXE Core) Laszlo Ersek
2020-11-19 10:53 ` [PATCH v2 RESEND 1/2] MdeModulePkg/Core/Dxe: assert SectionInstance invariant in FindChildNode() Laszlo Ersek
2020-11-19 10:53 ` Laszlo Ersek [this message]
2020-11-20 5:30 ` 回复: [edk2-devel] [PATCH v2 RESEND 0/2] security fix: unlimited FV recursion, round 2 (DXE Core) gaoliming
2020-11-20 10:57 ` Laszlo Ersek
2020-11-21 1:43 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201119105340.16225-3-lersek@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox