-
Notifications
You must be signed in to change notification settings - Fork 505
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
EDK2's Network Package is susceptible to an out-of-bounds read vulnerability when processing Neighbor Discovery Redirect message. This vulnerability can be exploited by an attacker to gain unauthorized access and potentially lead to a loss of Confidentiality. References: https://nvd.nist.gov/vuln/detail/CVE-2023-45231 Upstream-patches: tianocore/edk2@bbfee34 tianocore/edk2@6f77463 Signed-off-by: Soumya Sambu <[email protected]>
- Loading branch information
1 parent
50b5017
commit bdff14d
Showing
3 changed files
with
317 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
From bbfee34f4188ac00371abe1389ae9c9fb989a0cd Mon Sep 17 00:00:00 2001 | ||
From: Doug Flick <[email protected]> | ||
Date: Fri, 26 Jan 2024 05:54:48 +0800 | ||
Subject: [PATCH] NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45231 Patch | ||
|
||
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4536 | ||
|
||
Bug Overview: | ||
PixieFail Bug #3 | ||
CVE-2023-45231 | ||
CVSS 6.5 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N | ||
CWE-125 Out-of-bounds Read | ||
|
||
Out-of-bounds read when handling a ND Redirect message with truncated | ||
options | ||
|
||
Change Overview: | ||
|
||
Adds a check to prevent truncated options from being parsed | ||
+ // | ||
+ // Cannot process truncated options. | ||
+ // Cannot process options with a length of 0 as there is no Type | ||
field. | ||
+ // | ||
+ if (OptionLen < sizeof (IP6_OPTION_HEADER)) { | ||
+ return FALSE; | ||
+ } | ||
|
||
Cc: Saloni Kasbekar <[email protected]> | ||
Cc: Zachary Clark-williams <[email protected]> | ||
|
||
Signed-off-by: Doug Flick [MSFT] <[email protected]> | ||
Reviewed-by: Saloni Kasbekar <[email protected]> | ||
|
||
CVE: CVE-2023-45231 | ||
|
||
Upstream-Status: Backport [https://github.com/tianocore/edk2/commit/bbfee34f4188ac00371abe1389ae9c9fb989a0cd] | ||
|
||
Signed-off-by: Soumya Sambu <[email protected]> | ||
--- | ||
NetworkPkg/Ip6Dxe/Ip6Option.c | 8 ++++++++ | ||
1 file changed, 8 insertions(+) | ||
|
||
diff --git a/NetworkPkg/Ip6Dxe/Ip6Option.c b/NetworkPkg/Ip6Dxe/Ip6Option.c | ||
index 199eea124d..8718d5d875 100644 | ||
--- a/NetworkPkg/Ip6Dxe/Ip6Option.c | ||
+++ b/NetworkPkg/Ip6Dxe/Ip6Option.c | ||
@@ -137,6 +137,14 @@ Ip6IsNDOptionValid ( | ||
return FALSE; | ||
} | ||
|
||
+ // | ||
+ // Cannot process truncated options. | ||
+ // Cannot process options with a length of 0 as there is no Type field. | ||
+ // | ||
+ if (OptionLen < sizeof (IP6_OPTION_HEADER)) { | ||
+ return FALSE; | ||
+ } | ||
+ | ||
Offset = 0; | ||
|
||
// | ||
-- | ||
2.40.0 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,250 @@ | ||
From 6f77463d72807ec7f4ed6518c3dac29a1040df9f Mon Sep 17 00:00:00 2001 | ||
From: Doug Flick <[email protected]> | ||
Date: Fri, 26 Jan 2024 05:54:49 +0800 | ||
Subject: [PATCH] NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45231 Unit Tests | ||
|
||
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4536 | ||
|
||
Validates that the patch for... | ||
|
||
Out-of-bounds read when handling a ND Redirect message with truncated | ||
options | ||
|
||
.. has been fixed | ||
|
||
Tests the following function to ensure that an out of bounds read does | ||
not occur | ||
Ip6OptionValidation | ||
|
||
Cc: Saloni Kasbekar <[email protected]> | ||
Cc: Zachary Clark-williams <[email protected]> | ||
|
||
Signed-off-by: Doug Flick [MSFT] <[email protected]> | ||
Reviewed-by: Saloni Kasbekar <[email protected]> | ||
|
||
CVE: CVE-2023-45231 | ||
|
||
Upstream-Status: Backport [https://github.com/tianocore/edk2/commit/6f77463d72807ec7f4ed6518c3dac29a1040df9f] | ||
|
||
Signed-off-by: Soumya Sambu <[email protected]> | ||
--- | ||
.../Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp | 20 +++ | ||
.../Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf | 42 ++++++ | ||
.../Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp | 129 ++++++++++++++++++ | ||
3 files changed, 191 insertions(+) | ||
create mode 100644 NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp | ||
create mode 100644 NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf | ||
create mode 100644 NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp | ||
|
||
diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp | ||
new file mode 100644 | ||
index 0000000000..6ebfd5fdfb | ||
--- /dev/null | ||
+++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp | ||
@@ -0,0 +1,20 @@ | ||
+/** @file | ||
+ Acts as the main entry point for the tests for the Ip6Dxe module. | ||
+ | ||
+ Copyright (c) Microsoft Corporation | ||
+ SPDX-License-Identifier: BSD-2-Clause-Patent | ||
+**/ | ||
+#include <gtest/gtest.h> | ||
+ | ||
+//////////////////////////////////////////////////////////////////////////////// | ||
+// Run the tests | ||
+//////////////////////////////////////////////////////////////////////////////// | ||
+int | ||
+main ( | ||
+ int argc, | ||
+ char *argv[] | ||
+ ) | ||
+{ | ||
+ testing::InitGoogleTest (&argc, argv); | ||
+ return RUN_ALL_TESTS (); | ||
+} | ||
diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf | ||
new file mode 100644 | ||
index 0000000000..6e4de0745f | ||
--- /dev/null | ||
+++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf | ||
@@ -0,0 +1,42 @@ | ||
+## @file | ||
+# Unit test suite for the Ip6Dxe using Google Test | ||
+# | ||
+# Copyright (c) Microsoft Corporation.<BR> | ||
+# SPDX-License-Identifier: BSD-2-Clause-Patent | ||
+## | ||
+[Defines] | ||
+ INF_VERSION = 0x00010017 | ||
+ BASE_NAME = Ip6DxeUnitTest | ||
+ FILE_GUID = 4F05D17D-D3E7-4AAE-820C-576D46D2D34A | ||
+ VERSION_STRING = 1.0 | ||
+ MODULE_TYPE = HOST_APPLICATION | ||
+# | ||
+# The following information is for reference only and not required by the build tools. | ||
+# | ||
+# VALID_ARCHITECTURES = IA32 X64 AARCH64 | ||
+# | ||
+[Sources] | ||
+ Ip6DxeGoogleTest.cpp | ||
+ Ip6OptionGoogleTest.cpp | ||
+ ../Ip6Option.c | ||
+ | ||
+[Packages] | ||
+ MdePkg/MdePkg.dec | ||
+ MdeModulePkg/MdeModulePkg.dec | ||
+ UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec | ||
+ NetworkPkg/NetworkPkg.dec | ||
+ | ||
+[LibraryClasses] | ||
+ GoogleTestLib | ||
+ DebugLib | ||
+ NetLib | ||
+ PcdLib | ||
+ | ||
+[Protocols] | ||
+ gEfiDhcp6ServiceBindingProtocolGuid | ||
+ | ||
+[Pcd] | ||
+ gEfiNetworkPkgTokenSpaceGuid.PcdDhcp6UidType | ||
+ | ||
+[Guids] | ||
+ gZeroGuid | ||
diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp | ||
new file mode 100644 | ||
index 0000000000..f2cd90e1a9 | ||
--- /dev/null | ||
+++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp | ||
@@ -0,0 +1,129 @@ | ||
+/** @file | ||
+ Tests for Ip6Option.c. | ||
+ | ||
+ Copyright (c) Microsoft Corporation | ||
+ SPDX-License-Identifier: BSD-2-Clause-Patent | ||
+**/ | ||
+#include <gtest/gtest.h> | ||
+ | ||
+extern "C" { | ||
+ #include <Uefi.h> | ||
+ #include <Library/BaseLib.h> | ||
+ #include <Library/DebugLib.h> | ||
+ #include "../Ip6Impl.h" | ||
+ #include "../Ip6Option.h" | ||
+} | ||
+ | ||
+///////////////////////////////////////////////////////////////////////// | ||
+// Defines | ||
+/////////////////////////////////////////////////////////////////////// | ||
+ | ||
+#define IP6_PREFIX_INFO_OPTION_DATA_LEN 32 | ||
+#define OPTION_HEADER_IP6_PREFIX_DATA_LEN (sizeof (IP6_OPTION_HEADER) + IP6_PREFIX_INFO_OPTION_DATA_LEN) | ||
+ | ||
+//////////////////////////////////////////////////////////////////////// | ||
+// Symbol Definitions | ||
+// These functions are not directly under test - but required to compile | ||
+//////////////////////////////////////////////////////////////////////// | ||
+UINT32 mIp6Id; | ||
+ | ||
+EFI_STATUS | ||
+Ip6SendIcmpError ( | ||
+ IN IP6_SERVICE *IpSb, | ||
+ IN NET_BUF *Packet, | ||
+ IN EFI_IPv6_ADDRESS *SourceAddress OPTIONAL, | ||
+ IN EFI_IPv6_ADDRESS *DestinationAddress, | ||
+ IN UINT8 Type, | ||
+ IN UINT8 Code, | ||
+ IN UINT32 *Pointer OPTIONAL | ||
+ ) | ||
+{ | ||
+ // .. | ||
+ return EFI_SUCCESS; | ||
+} | ||
+ | ||
+//////////////////////////////////////////////////////////////////////// | ||
+// Ip6OptionValidation Tests | ||
+//////////////////////////////////////////////////////////////////////// | ||
+ | ||
+// Define a fixture for your tests if needed | ||
+class Ip6OptionValidationTest : public ::testing::Test { | ||
+protected: | ||
+ // Add any setup code if needed | ||
+ virtual void | ||
+ SetUp ( | ||
+ ) | ||
+ { | ||
+ // Initialize any resources or variables | ||
+ } | ||
+ | ||
+ // Add any cleanup code if needed | ||
+ virtual void | ||
+ TearDown ( | ||
+ ) | ||
+ { | ||
+ // Clean up any resources or variables | ||
+ } | ||
+}; | ||
+ | ||
+// Test Description: | ||
+// Null option should return false | ||
+TEST_F (Ip6OptionValidationTest, NullOptionShouldReturnFalse) { | ||
+ UINT8 *option = nullptr; | ||
+ UINT16 optionLen = 10; // Provide a suitable length | ||
+ | ||
+ EXPECT_FALSE (Ip6IsNDOptionValid (option, optionLen)); | ||
+} | ||
+ | ||
+// Test Description: | ||
+// Truncated option should return false | ||
+TEST_F (Ip6OptionValidationTest, TruncatedOptionShouldReturnFalse) { | ||
+ UINT8 option[] = { 0x01 }; // Provide a truncated option | ||
+ UINT16 optionLen = 1; | ||
+ | ||
+ EXPECT_FALSE (Ip6IsNDOptionValid (option, optionLen)); | ||
+} | ||
+ | ||
+// Test Description: | ||
+// Ip6OptionPrefixInfo Option with zero length should return false | ||
+TEST_F (Ip6OptionValidationTest, OptionWithZeroLengthShouldReturnFalse) { | ||
+ IP6_OPTION_HEADER optionHeader; | ||
+ | ||
+ optionHeader.Type = Ip6OptionPrefixInfo; | ||
+ optionHeader.Length = 0; | ||
+ UINT8 option[sizeof (IP6_OPTION_HEADER)]; | ||
+ | ||
+ CopyMem (option, &optionHeader, sizeof (IP6_OPTION_HEADER)); | ||
+ UINT16 optionLen = sizeof (IP6_OPTION_HEADER); | ||
+ | ||
+ EXPECT_FALSE (Ip6IsNDOptionValid (option, optionLen)); | ||
+} | ||
+ | ||
+// Test Description: | ||
+// Ip6OptionPrefixInfo Option with valid length should return true | ||
+TEST_F (Ip6OptionValidationTest, ValidPrefixInfoOptionShouldReturnTrue) { | ||
+ IP6_OPTION_HEADER optionHeader; | ||
+ | ||
+ optionHeader.Type = Ip6OptionPrefixInfo; | ||
+ optionHeader.Length = 4; // Length 4 * 8 = 32 | ||
+ UINT8 option[OPTION_HEADER_IP6_PREFIX_DATA_LEN]; | ||
+ | ||
+ CopyMem (option, &optionHeader, sizeof (IP6_OPTION_HEADER)); | ||
+ | ||
+ EXPECT_TRUE (Ip6IsNDOptionValid (option, IP6_PREFIX_INFO_OPTION_DATA_LEN)); | ||
+} | ||
+ | ||
+// Test Description: | ||
+// Ip6OptionPrefixInfo Option with invalid length should return false | ||
+TEST_F (Ip6OptionValidationTest, InvalidPrefixInfoOptionLengthShouldReturnFalse) { | ||
+ IP6_OPTION_HEADER optionHeader; | ||
+ | ||
+ optionHeader.Type = Ip6OptionPrefixInfo; | ||
+ optionHeader.Length = 3; // Length 3 * 8 = 24 (Invalid) | ||
+ UINT8 option[sizeof (IP6_OPTION_HEADER)]; | ||
+ | ||
+ CopyMem (option, &optionHeader, sizeof (IP6_OPTION_HEADER)); | ||
+ UINT16 optionLen = sizeof (IP6_OPTION_HEADER); | ||
+ | ||
+ EXPECT_FALSE (Ip6IsNDOptionValid (option, optionLen)); | ||
+} | ||
-- | ||
2.40.0 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters