diff mbox series

[scarthgap,V2,1/2] ovmf: fix CVE-2024-38796

Message ID 20241126032653.326194-1-hongxu.jia@windriver.com
State Accepted
Delegated to: Steve Sakoman
Headers show
Series [scarthgap,V2,1/2] ovmf: fix CVE-2024-38796 | expand

Commit Message

Hongxu Jia Nov. 26, 2024, 3:26 a.m. UTC 
Backport fix from upstream to resolve CVE-2024-38796

  https://github.com/tianocore/edk2/commit/c95233b8525ca6828921affd1496146cff262e65

Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
---
 ...-Fix-overflow-issue-in-BasePeCoffLib.patch | 36 +++++++++++++++++++
 meta/recipes-core/ovmf/ovmf_git.bb            |  1 +
 2 files changed, 37 insertions(+)
 create mode 100644 meta/recipes-core/ovmf/ovmf/0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch

Comments

Steve Sakoman Nov. 27, 2024, 6:24 p.m. UTC | #1 
I'm also getting build time errors on the scarthgap version (also
seems to be line ending related):

https://errors.yoctoproject.org/Errors/Details/836555/

Steve

On Mon, Nov 25, 2024 at 7:26 PM Hongxu Jia <hongxu.jia@windriver.com> wrote:
>
> Backport fix from upstream to resolve CVE-2024-38796
>
>   https://github.com/tianocore/edk2/commit/c95233b8525ca6828921affd1496146cff262e65
>
> Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
> ---
>  ...-Fix-overflow-issue-in-BasePeCoffLib.patch | 36 +++++++++++++++++++
>  meta/recipes-core/ovmf/ovmf_git.bb            |  1 +
>  2 files changed, 37 insertions(+)
>  create mode 100644 meta/recipes-core/ovmf/ovmf/0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch
>
> diff --git a/meta/recipes-core/ovmf/ovmf/0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch b/meta/recipes-core/ovmf/ovmf/0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch
> new file mode 100644
> index 0000000000..c6e15c5069
> --- /dev/null
> +++ b/meta/recipes-core/ovmf/ovmf/0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch
> @@ -0,0 +1,36 @@
> +From 5f7bd3f3c4747d5bb2733f017f8c5b93b63a74e3 Mon Sep 17 00:00:00 2001
> +From: Doug Flick <dougflick@microsoft.com>
> +Date: Fri, 22 Nov 2024 13:03:33 +0800
> +Subject: [PATCH] MdePkg: Fix overflow issue in BasePeCoffLib
> +
> +The RelocDir->Size is a UINT32 value, and RelocDir->VirtualAddress is
> +also a UINT32 value. The current code does not check for overflow when
> +adding RelocDir->Size to RelocDir->VirtualAddress. This patch adds a
> +check to ensure that the addition does not overflow.
> +
> +Signed-off-by: Doug Flick <dougflick@microsoft.com>
> +Authored-by: sriraamx gobichettipalayam <sri..@intel.com>
> +
> +CVE: CVE-2024-38796
> +Upstream-Status: Backport [https://github.com/tianocore/edk2/commit/c95233b8525ca6828921affd1496146cff262e65]
> +Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
> +---
> + MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> +index 86ff2e7..128090d 100644
> +--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> ++++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> +@@ -1054,7 +1054,7 @@ PeCoffLoaderRelocateImage (
> +     RelocDir = &Hdr.Te->DataDirectory[0];
> +   }
> +
> +-  if ((RelocDir != NULL) && (RelocDir->Size > 0)) {
> ++  if ((RelocDir != NULL) && (RelocDir->Size > 0) && (RelocDir->Size - 1 < MAX_UINT32 - RelocDir->VirtualAddress)) {
> +     RelocBase    = (EFI_IMAGE_BASE_RELOCATION *)PeCoffLoaderImageAddress (ImageContext, RelocDir->VirtualAddress, TeStrippedOffset);
> +     RelocBaseEnd = (EFI_IMAGE_BASE_RELOCATION *)PeCoffLoaderImageAddress (
> +                                                   ImageContext,
> +--
> +2.34.1
> +
> diff --git a/meta/recipes-core/ovmf/ovmf_git.bb b/meta/recipes-core/ovmf/ovmf_git.bb
> index 35ca8d1834..cc2ac4268c 100644
> --- a/meta/recipes-core/ovmf/ovmf_git.bb
> +++ b/meta/recipes-core/ovmf/ovmf_git.bb
> @@ -24,6 +24,7 @@ SRC_URI = "gitsm://github.com/tianocore/edk2.git;branch=master;protocol=https \
>             file://0002-BaseTools-makefile-adjust-to-build-in-under-bitbake.patch \
>             file://0003-debug-prefix-map.patch \
>             file://0004-reproducible.patch \
> +           file://0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch \
>             "
>
>  PV = "edk2-stable202402"
> --
> 2.25.1
>
Hongxu Jia Nov. 28, 2024, 8:14 a.m. UTC | #2 
Hi Steve,

I am afraid the issue was caused by  CR("^M") in ovmf source code

The source of ovmf use CR (^M) as new line, we should use 'git am --keep-cr xxxx.patch' to apply the patch,
otherwise do_patch failed

But I do not know how you apply the patch to your build or via patchtest automatically, do you use git am with option --keep-cr?


//Hongxu
________________________________
From: Steve Sakoman <steve@sakoman.com>
Sent: Thursday, November 28, 2024 2:24 AM
To: Jia, Hongxu <Hongxu.Jia@windriver.com>
Cc: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org>
Subject: Re: [scarthgap][PATCH V2 1/2] ovmf: fix CVE-2024-38796

CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know the content is safe.

I'm also getting build time errors on the scarthgap version (also
seems to be line ending related):

https://errors.yoctoproject.org/Errors/Details/836555/

Steve

On Mon, Nov 25, 2024 at 7:26 PM Hongxu Jia <hongxu.jia@windriver.com> wrote:
>
> Backport fix from upstream to resolve CVE-2024-38796
>
>   https://github.com/tianocore/edk2/commit/c95233b8525ca6828921affd1496146cff262e65
>
> Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
> ---
>  ...-Fix-overflow-issue-in-BasePeCoffLib.patch | 36 +++++++++++++++++++
>  meta/recipes-core/ovmf/ovmf_git.bb            |  1 +
>  2 files changed, 37 insertions(+)
>  create mode 100644 meta/recipes-core/ovmf/ovmf/0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch
>
> diff --git a/meta/recipes-core/ovmf/ovmf/0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch b/meta/recipes-core/ovmf/ovmf/0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch
> new file mode 100644
> index 0000000000..c6e15c5069
> --- /dev/null
> +++ b/meta/recipes-core/ovmf/ovmf/0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch
> @@ -0,0 +1,36 @@
> +From 5f7bd3f3c4747d5bb2733f017f8c5b93b63a74e3 Mon Sep 17 00:00:00 2001
> +From: Doug Flick <dougflick@microsoft.com>
> +Date: Fri, 22 Nov 2024 13:03:33 +0800
> +Subject: [PATCH] MdePkg: Fix overflow issue in BasePeCoffLib
> +
> +The RelocDir->Size is a UINT32 value, and RelocDir->VirtualAddress is
> +also a UINT32 value. The current code does not check for overflow when
> +adding RelocDir->Size to RelocDir->VirtualAddress. This patch adds a
> +check to ensure that the addition does not overflow.
> +
> +Signed-off-by: Doug Flick <dougflick@microsoft.com>
> +Authored-by: sriraamx gobichettipalayam <sri..@intel.com>
> +
> +CVE: CVE-2024-38796
> +Upstream-Status: Backport [https://github.com/tianocore/edk2/commit/c95233b8525ca6828921affd1496146cff262e65]
> +Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
> +---
> + MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> +index 86ff2e7..128090d 100644
> +--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> ++++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> +@@ -1054,7 +1054,7 @@ PeCoffLoaderRelocateImage (
> +     RelocDir = &Hdr.Te->DataDirectory[0];
> +   }
> +
> +-  if ((RelocDir != NULL) && (RelocDir->Size > 0)) {
> ++  if ((RelocDir != NULL) && (RelocDir->Size > 0) && (RelocDir->Size - 1 < MAX_UINT32 - RelocDir->VirtualAddress)) {
> +     RelocBase    = (EFI_IMAGE_BASE_RELOCATION *)PeCoffLoaderImageAddress (ImageContext, RelocDir->VirtualAddress, TeStrippedOffset);
> +     RelocBaseEnd = (EFI_IMAGE_BASE_RELOCATION *)PeCoffLoaderImageAddress (
> +                                                   ImageContext,
> +--
> +2.34.1
> +
> diff --git a/meta/recipes-core/ovmf/ovmf_git.bb b/meta/recipes-core/ovmf/ovmf_git.bb
> index 35ca8d1834..cc2ac4268c 100644
> --- a/meta/recipes-core/ovmf/ovmf_git.bb
> +++ b/meta/recipes-core/ovmf/ovmf_git.bb
> @@ -24,6 +24,7 @@ SRC_URI = "gitsm://github.com/tianocore/edk2.git;branch=master;protocol=https \
>             file://0002-BaseTools-makefile-adjust-to-build-in-under-bitbake.patch \
>             file://0003-debug-prefix-map.patch \
>             file://0004-reproducible.patch \
> +           file://0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch \
>             "
>
>  PV = "edk2-stable202402"
> --
> 2.25.1
>
diff mbox series

Patch

diff --git a/meta/recipes-core/ovmf/ovmf/0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch b/meta/recipes-core/ovmf/ovmf/0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch
new file mode 100644
index 0000000000..c6e15c5069
--- /dev/null
+++ b/meta/recipes-core/ovmf/ovmf/0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch
@@ -0,0 +1,36 @@ 
+From 5f7bd3f3c4747d5bb2733f017f8c5b93b63a74e3 Mon Sep 17 00:00:00 2001
+From: Doug Flick <dougflick@microsoft.com>
+Date: Fri, 22 Nov 2024 13:03:33 +0800
+Subject: [PATCH] MdePkg: Fix overflow issue in BasePeCoffLib
+
+The RelocDir->Size is a UINT32 value, and RelocDir->VirtualAddress is
+also a UINT32 value. The current code does not check for overflow when
+adding RelocDir->Size to RelocDir->VirtualAddress. This patch adds a
+check to ensure that the addition does not overflow.
+
+Signed-off-by: Doug Flick <dougflick@microsoft.com>
+Authored-by: sriraamx gobichettipalayam <sri..@intel.com>
+
+CVE: CVE-2024-38796
+Upstream-Status: Backport [https://github.com/tianocore/edk2/commit/c95233b8525ca6828921affd1496146cff262e65]
+Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
+---
+ MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+index 86ff2e7..128090d 100644
+--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
++++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+@@ -1054,7 +1054,7 @@ PeCoffLoaderRelocateImage (
+     RelocDir = &Hdr.Te->DataDirectory[0];
+   }
+ 
+-  if ((RelocDir != NULL) && (RelocDir->Size > 0)) {
++  if ((RelocDir != NULL) && (RelocDir->Size > 0) && (RelocDir->Size - 1 < MAX_UINT32 - RelocDir->VirtualAddress)) {
+     RelocBase    = (EFI_IMAGE_BASE_RELOCATION *)PeCoffLoaderImageAddress (ImageContext, RelocDir->VirtualAddress, TeStrippedOffset);
+     RelocBaseEnd = (EFI_IMAGE_BASE_RELOCATION *)PeCoffLoaderImageAddress (
+                                                   ImageContext,
+-- 
+2.34.1
+
diff --git a/meta/recipes-core/ovmf/ovmf_git.bb b/meta/recipes-core/ovmf/ovmf_git.bb
index 35ca8d1834..cc2ac4268c 100644
--- a/meta/recipes-core/ovmf/ovmf_git.bb
+++ b/meta/recipes-core/ovmf/ovmf_git.bb
@@ -24,6 +24,7 @@  SRC_URI = "gitsm://github.com/tianocore/edk2.git;branch=master;protocol=https \
            file://0002-BaseTools-makefile-adjust-to-build-in-under-bitbake.patch \
            file://0003-debug-prefix-map.patch \
            file://0004-reproducible.patch \
+           file://0001-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch \
            "
 
 PV = "edk2-stable202402"