From 46bc7748285f9254e2bc6d12553d91064977a0ca Mon Sep 17 00:00:00 2001 From: Jiajie Li Date: Mon, 28 Jun 2021 19:09:23 +0800 Subject: [PATCH] Fix CVE-2021-28210 Signed-off-by: Jiajie Li --- ...nInstance-invariant-in-FindChildNode.patch | 106 +++++++++ ...wVol-encapsulation-section-recursion.patch | 202 ++++++++++++++++++ edk2.spec | 7 +- 3 files changed, 314 insertions(+), 1 deletion(-) create mode 100755 0004-MdeModulePkg-Core-Dxe-assert-SectionInstance-invariant-in-FindChildNode.patch create mode 100755 0005-MdeModulePkg-Core-Dxe-limit-FwVol-encapsulation-section-recursion.patch diff --git a/0004-MdeModulePkg-Core-Dxe-assert-SectionInstance-invariant-in-FindChildNode.patch b/0004-MdeModulePkg-Core-Dxe-assert-SectionInstance-invariant-in-FindChildNode.patch new file mode 100755 index 0000000..8498725 --- /dev/null +++ b/0004-MdeModulePkg-Core-Dxe-assert-SectionInstance-invariant-in-FindChildNode.patch @@ -0,0 +1,106 @@ +From 4ea70df0973caf3763aa306e8d6571fc37aa35e5 Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Mon, 28 Sep 2020 16:29:01 +0200 +Subject: [PATCH v2 1/2] MdeModulePkg/Core/Dxe: assert SectionInstance + invariant in FindChildNode() +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +FindChildNode() has two callers: GetSection(), and FindChildNode() itself. + +- At the GetSection() call site, a positive (i.e., nonzero) + SectionInstance is passed. This is because GetSection() takes a + zero-based (UINTN) SectionInstance, and then passes + Instance=(SectionInstance+1) to FindChildNode(). + +- For reaching the recursive FindChildNode() call site, a section type + mismatch, or a section instance mismatch, is necessary. This means, + respectively, that SectionInstance will either not have been decreased, + or not to zero anyway, at the recursive FindChildNode() call site. + +Add two ASSERT()s to FindChildNode(), for expressing the (SectionSize>0) +invariant. + +In turn, the invariant provides the explanation why, after the recursive +call, a zero SectionInstance implies success. Capture it in a comment. + +Cc: Dandan Bi +Cc: Hao A Wu +Cc: Jian J Wang +Cc: Liming Gao +Cc: Philippe Mathieu-Daudé +Signed-off-by: Laszlo Ersek +--- + +Notes: + v2: + - no change + + MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c | 23 +++++++++++++++----- + 1 file changed, 17 insertions(+), 6 deletions(-) + +diff --git a/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c b/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c +index d678166db475..d7f7ef427422 100644 +--- a/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c ++++ b/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c +@@ -952,8 +952,8 @@ CreateChildNode ( + search. + @param SearchType Indicates the type of section to search for. + @param SectionInstance Indicates which instance of section to find. +- This is an in/out parameter to deal with +- recursions. ++ This is an in/out parameter and it is 1-based, ++ to deal with recursions. + @param SectionDefinitionGuid Guid of section definition + @param FoundChild Output indicating the child node that is found. + @param FoundStream Output indicating which section stream the child +@@ -988,6 +988,8 @@ FindChildNode ( + EFI_STATUS ErrorStatus; + EFI_STATUS Status; + ++ ASSERT (*SectionInstance > 0); ++ + CurrentChildNode = NULL; + ErrorStatus = EFI_NOT_FOUND; + +@@ -1037,6 +1039,11 @@ FindChildNode ( + } + } + ++ // ++ // Type mismatch, or we haven't found the desired instance yet. ++ // ++ ASSERT (*SectionInstance > 0); ++ + if (CurrentChildNode->EncapsulatedStreamHandle != NULL_STREAM_HANDLE) { + // + // If the current node is an encapsulating node, recurse into it... +@@ -1050,16 +1057,20 @@ FindChildNode ( + &RecursedFoundStream, + AuthenticationStatus + ); +- // +- // If the status is not EFI_SUCCESS, just save the error code and continue +- // to find the request child node in the rest stream. +- // + if (*SectionInstance == 0) { ++ // ++ // The recursive FindChildNode() call decreased (*SectionInstance) to ++ // zero. ++ // + ASSERT_EFI_ERROR (Status); + *FoundChild = RecursedChildNode; + *FoundStream = RecursedFoundStream; + return EFI_SUCCESS; + } else { ++ // ++ // If the status is not EFI_SUCCESS, just save the error code and ++ // continue to find the request child node in the rest stream. ++ // + ErrorStatus = Status; + } + } else if ((CurrentChildNode->Type == EFI_SECTION_GUID_DEFINED) && (SearchType != EFI_SECTION_GUID_DEFINED)) { +-- +2.19.1.3.g30247aa5d201 + diff --git a/0005-MdeModulePkg-Core-Dxe-limit-FwVol-encapsulation-section-recursion.patch b/0005-MdeModulePkg-Core-Dxe-limit-FwVol-encapsulation-section-recursion.patch new file mode 100755 index 0000000..5960c3d --- /dev/null +++ b/0005-MdeModulePkg-Core-Dxe-limit-FwVol-encapsulation-section-recursion.patch @@ -0,0 +1,202 @@ +From 5d02b0176fb8584e44c1b8f2bc1f934e23b017ed Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Mon, 28 Sep 2020 15:02:02 +0200 +Subject: [PATCH v2 2/2] MdeModulePkg/Core/Dxe: limit FwVol encapsulation + section recursion +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +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 +Cc: Hao A Wu +Cc: Jian J Wang +Cc: Liming Gao +Cc: Philippe Mathieu-Daudé +Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1743 +Signed-off-by: Laszlo Ersek +--- + +Notes: + v2: + - change the DEC default of the new PCD + (PcdFwVolDxeMaxEncapsulationDepth) from 8 to 16 (0x10) [Liming] + + MdeModulePkg/MdeModulePkg.dec | 6 ++++ + MdeModulePkg/MdeModulePkg.uni | 6 ++++ + MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + + MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c | 33 ++++++++++++++++++-- + 4 files changed, 44 insertions(+), 2 deletions(-) + +diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec +index cb30a7975849..eac19a6edcc5 100644 +--- a/MdeModulePkg/MdeModulePkg.dec ++++ b/MdeModulePkg/MdeModulePkg.dec +@@ -1505,6 +1505,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 b8c867379a86..9b1be3220fad 100644 +--- a/MdeModulePkg/MdeModulePkg.uni ++++ b/MdeModulePkg/MdeModulePkg.uni +@@ -1153,6 +1153,12 @@ + "Note:
" + "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,
" ++ "in the DXE phase. Minimum value is 1. Sections nested more deeply are
" ++ "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.

" +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 +diff --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 + diff --git a/edk2.spec b/edk2.spec index 23edbcc..a407c25 100644 --- a/edk2.spec +++ b/edk2.spec @@ -5,7 +5,7 @@ Name: edk2 Version: %{stable_date} -Release: 3 +Release: 4 Summary: EFI Development Kit II License: BSD-2-Clause-Patent URL: https://github.com/tianocore/edk2 @@ -17,6 +17,8 @@ Patch0002: 0002-CryptoPkg-Upgrade-OpenSSL-to-1.1.1f.patch Patch0003: 0001-SecurityPkg-DxeImageVerificationLib-extract-SecDataD.patch Patch0004: 0002-SecurityPkg-DxeImageVerificationLib-assign-WinCertif.patch Patch0005: 0003-SecurityPkg-DxeImageVerificationLib-catch-alignment-.patch +Patch0006: 0004-MdeModulePkg-Core-Dxe-assert-SectionInstance-invariant-in-FindChildNode.patch +Patch0007: 0005-MdeModulePkg-Core-Dxe-limit-FwVol-encapsulation-section-recursion.patch BuildRequires: acpica-tools gcc gcc-c++ libuuid-devel python3 bc nasm python2 @@ -212,6 +214,9 @@ chmod +x %{buildroot}%{_bindir}/Rsa2048Sha256GenerateKeys %endif %changelog +* Mon Jun 28 2021 Jiajie Li - 202002-4 +- Fix CVE-2021-28210 + * Wed 19 May 2021 openEuler Buildteam - 202002-3 Fix CVE-2019-14562 -- Gitee