Hello community,
here is the log from the commit of package ovmf for openSUSE:Leap:15.2 checked
in at 2020-04-14 14:20:16
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Comparing /work/SRC/openSUSE:Leap:15.2/ovmf (Old)
and /work/SRC/openSUSE:Leap:15.2/.ovmf.new.3248 (New)
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "ovmf"
Tue Apr 14 14:20:16 2020 rev:49 rq:792539 version:201911
Changes:
--------
--- /work/SRC/openSUSE:Leap:15.2/ovmf/ovmf.changes 2020-03-01
08:51:33.797295856 +0100
+++ /work/SRC/openSUSE:Leap:15.2/.ovmf.new.3248/ovmf.changes 2020-04-14
14:20:21.245246409 +0200
@@ -1,0 +2,6 @@
+Mon Apr 6 03:54:48 UTC 2020 - Gary Ching-Pang Lin <g...@suse.com>
+
+- Add ovmf-bsc1163927-fix-ping-and-ip6dxe.patch to fix crash and
+ hang in ShellPkg and Ip6Dxe (bsc#1163927, CVE-2019-14559)
+
+-------------------------------------------------------------------
New:
----
ovmf-bsc1163927-fix-ping-and-ip6dxe.patch
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Other differences:
------------------
++++++ ovmf.spec ++++++
--- /var/tmp/diff_new_pack.5cqSPg/_old 2020-04-14 14:20:22.029246994 +0200
+++ /var/tmp/diff_new_pack.5cqSPg/_new 2020-04-14 14:20:22.029246994 +0200
@@ -52,6 +52,7 @@
Patch6:
%{name}-bsc1163959-PiDxeS3BootScriptLib-fix-numeric-truncation.patch
Patch7: %{name}-bsc1163969-fix-DxeImageVerificationHandler.patch
Patch8: %{name}-bsc1163927-fix-ip4dxe-and-arpdxe.patch
+Patch9: %{name}-bsc1163927-fix-ping-and-ip6dxe.patch
Patch100: openssl-fix-syntax-error.patch
BuildRoot: %{_tmppath}/%{name}-%{version}-build
BuildRequires: bc
@@ -178,6 +179,7 @@
%patch6 -p1
%patch7 -p1
%patch8 -p1
+%patch9 -p1
# add openssl
pushd CryptoPkg/Library/OpensslLib/openssl
++++++ ovmf-bsc1163927-fix-ping-and-ip6dxe.patch ++++++
>From 6a5e9bdd108741bcc8fd68276116f41b4a35da75 Mon Sep 17 00:00:00 2001
From: Maciej Rabeda <maciej.rab...@linux.intel.com>
Date: Thu, 27 Feb 2020 11:30:43 +0100
Subject: [PATCH 1/3] ShellPkg: Fix 'ping' command Ip4 receive flow.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2032
'ping' command's receive flow utilizes a single Rx token which it
attempts to reuse before recycling the previously received packet.
This causes a situation where under ICMP traffic,
Ping6OnEchoReplyReceived() function will receive an already
recycled packet with EFI_SUCCESS token status and finally
dereference invalid pointers from RxData structure.
Cc: Ray Ni <ray...@intel.com>
Cc: Zhichao Gao <zhichao....@intel.com>
Signed-off-by: Maciej Rabeda <maciej.rab...@linux.intel.com>
Reviewed-by: Siyuan Fu <siyuan...@intel.com>
Acked-by: Zhichao Gao <zhichao....@intel.com>
(cherry picked from commit 65c73df44c61235ede84c5aa1d2eab6650844966)
---
ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
index 23567fa2c1bb..a3fa32515192 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
@@ -614,6 +614,11 @@ Ping6OnEchoReplyReceived (
ON_EXIT:
+ //
+ // Recycle the packet before reusing RxToken
+ //
+ gBS->SignalEvent (Private->IpChoice ==
PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->RecycleSignal);
+
if (Private->RxCount < Private->SendNum) {
//
// Continue to receive icmp echo reply packets.
@@ -632,10 +637,6 @@ ON_EXIT:
//
Private->Status = EFI_SUCCESS;
}
- //
- // Singal to recycle the each rxdata here, not at the end of process.
- //
- gBS->SignalEvent (Private->IpChoice ==
PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->RecycleSignal);
}
/**
--
2.25.1
>From 35fb4bd10b630663d7eaa6731e15089f2d6091b1 Mon Sep 17 00:00:00 2001
From: Maciej Rabeda <maciej.rab...@linux.intel.com>
Date: Mon, 2 Mar 2020 13:25:20 +0100
Subject: [PATCH 2/3] NetworkPkg/Ip6Dxe: Improve Neightbor Discovery message
validation.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2174
Problem has been identified with Ip6ProcessRouterAdvertise() when
Router Advertise packet contains options with malicious/invalid
'Length' field. This can lead to platform entering infinite loop
when processing options from that packet.
Cc: Jiaxin Wu <jiaxin...@intel.com>
Cc: Siyuan Fu <siyuan...@intel.com>
Signed-off-by: Maciej Rabeda <maciej.rab...@linux.intel.com>
Reviewed-by: Siyuan Fu <siyuan...@intel.com>
(cherry picked from commit 9c20342eed70ec99ec50cd73cb81804299f05403)
---
NetworkPkg/Ip6Dxe/Ip6Nd.c | 44 ++++++++++++++++-----------
NetworkPkg/Ip6Dxe/Ip6Nd.h | 13 ++++++++
NetworkPkg/Ip6Dxe/Ip6Option.c | 57 ++++++++++++++++++++++++++---------
3 files changed, 83 insertions(+), 31 deletions(-)
diff --git a/NetworkPkg/Ip6Dxe/Ip6Nd.c b/NetworkPkg/Ip6Dxe/Ip6Nd.c
index 67d7022a7673..1254f0fdd921 100644
--- a/NetworkPkg/Ip6Dxe/Ip6Nd.c
+++ b/NetworkPkg/Ip6Dxe/Ip6Nd.c
@@ -1927,7 +1927,7 @@ Ip6ProcessRouterAdvertise (
UINT32 ReachableTime;
UINT32 RetransTimer;
UINT16 RouterLifetime;
- UINT16 Offset;
+ UINT32 Offset;
UINT8 Type;
UINT8 Length;
IP6_ETHER_ADDR_OPTION LinkLayerOption;
@@ -2094,10 +2094,11 @@ Ip6ProcessRouterAdvertise (
//
// The only defined options that may appear are the Source
// Link-Layer Address, Prefix information and MTU options.
- // All included options have a length that is greater than zero.
+ // All included options have a length that is greater than zero and
+ // fit within the input packet.
//
Offset = 16;
- while (Offset < Head->PayloadLength) {
+ while (Offset < (UINT32) Head->PayloadLength) {
NetbufCopy (Packet, Offset, sizeof (UINT8), &Type);
switch (Type) {
case Ip6OptionEtherSource:
@@ -2105,9 +2106,12 @@ Ip6ProcessRouterAdvertise (
// Update the neighbor cache
//
NetbufCopy (Packet, Offset, sizeof (IP6_ETHER_ADDR_OPTION), (UINT8 *)
&LinkLayerOption);
- if (LinkLayerOption.Length <= 0) {
- goto Exit;
- }
+
+ //
+ // Option size validity ensured by Ip6IsNDOptionValid().
+ //
+ ASSERT (LinkLayerOption.Length != 0);
+ ASSERT (Offset + (UINT32) LinkLayerOption.Length * 8 >= (UINT32)
Head->PayloadLength);
ZeroMem (&LinkLayerAddress, sizeof (EFI_MAC_ADDRESS));
CopyMem (&LinkLayerAddress, LinkLayerOption.EtherAddr, 6);
@@ -2151,13 +2155,17 @@ Ip6ProcessRouterAdvertise (
}
}
- Offset = (UINT16) (Offset + (UINT16) LinkLayerOption.Length * 8);
+ Offset += (UINT32) LinkLayerOption.Length * 8;
break;
case Ip6OptionPrefixInfo:
NetbufCopy (Packet, Offset, sizeof (IP6_PREFIX_INFO_OPTION), (UINT8 *)
&PrefixOption);
- if (PrefixOption.Length != 4) {
- goto Exit;
- }
+
+ //
+ // Option size validity ensured by Ip6IsNDOptionValid().
+ //
+ ASSERT (PrefixOption.Length == 4);
+ ASSERT (Offset + (UINT32) PrefixOption.Length * 8 >= (UINT32)
Head->PayloadLength);
+
PrefixOption.ValidLifetime = NTOHL (PrefixOption.ValidLifetime);
PrefixOption.PreferredLifetime = NTOHL (PrefixOption.PreferredLifetime);
@@ -2321,9 +2329,12 @@ Ip6ProcessRouterAdvertise (
break;
case Ip6OptionMtu:
NetbufCopy (Packet, Offset, sizeof (IP6_MTU_OPTION), (UINT8 *)
&MTUOption);
- if (MTUOption.Length != 1) {
- goto Exit;
- }
+
+ //
+ // Option size validity ensured by Ip6IsNDOptionValid().
+ //
+ ASSERT (MTUOption.Length == 1);
+ ASSERT (Offset + (UINT32) MTUOption.Length * 8 >= (UINT32)
Head->PayloadLength);
//
// Use IPv6 minimum link MTU 1280 bytes as the maximum packet size in
order
@@ -2338,11 +2349,10 @@ Ip6ProcessRouterAdvertise (
// Silently ignore unrecognized options
//
NetbufCopy (Packet, Offset + sizeof (UINT8), sizeof (UINT8), &Length);
- if (Length <= 0) {
- goto Exit;
- }
- Offset = (UINT16) (Offset + (UINT16) Length * 8);
+ ASSERT (Length != 0);
+
+ Offset += (UINT32) Length * 8;
break;
}
}
diff --git a/NetworkPkg/Ip6Dxe/Ip6Nd.h b/NetworkPkg/Ip6Dxe/Ip6Nd.h
index 891a32d7d3ca..e4004662951a 100644
--- a/NetworkPkg/Ip6Dxe/Ip6Nd.h
+++ b/NetworkPkg/Ip6Dxe/Ip6Nd.h
@@ -56,12 +56,21 @@ VOID
VOID *Context
);
+typedef struct _IP6_OPTION_HEADER {
+ UINT8 Type;
+ UINT8 Length;
+} IP6_OPTION_HEADER;
+
+STATIC_ASSERT (sizeof (IP6_OPTION_HEADER) == 2, "IP6_OPTION_HEADER is expected
to be exactly 2 bytes long.");
+
typedef struct _IP6_ETHE_ADDR_OPTION {
UINT8 Type;
UINT8 Length;
UINT8 EtherAddr[6];
} IP6_ETHER_ADDR_OPTION;
+STATIC_ASSERT (sizeof (IP6_ETHER_ADDR_OPTION) == 8, "IP6_ETHER_ADDR_OPTION is
expected to be exactly 8 bytes long.");
+
typedef struct _IP6_MTU_OPTION {
UINT8 Type;
UINT8 Length;
@@ -69,6 +78,8 @@ typedef struct _IP6_MTU_OPTION {
UINT32 Mtu;
} IP6_MTU_OPTION;
+STATIC_ASSERT (sizeof (IP6_MTU_OPTION) == 8, "IP6_MTU_OPTION is expected to be
exactly 8 bytes long.");
+
typedef struct _IP6_PREFIX_INFO_OPTION {
UINT8 Type;
UINT8 Length;
@@ -80,6 +91,8 @@ typedef struct _IP6_PREFIX_INFO_OPTION {
EFI_IPv6_ADDRESS Prefix;
} IP6_PREFIX_INFO_OPTION;
+STATIC_ASSERT (sizeof (IP6_PREFIX_INFO_OPTION) == 32, "IP6_PREFIX_INFO_OPTION
is expected to be exactly 32 bytes long.");
+
typedef
VOID
(*IP6_DAD_CALLBACK) (
diff --git a/NetworkPkg/Ip6Dxe/Ip6Option.c b/NetworkPkg/Ip6Dxe/Ip6Option.c
index 4d92a852dc86..6b4b029d1479 100644
--- a/NetworkPkg/Ip6Dxe/Ip6Option.c
+++ b/NetworkPkg/Ip6Dxe/Ip6Option.c
@@ -129,45 +129,74 @@ Ip6IsNDOptionValid (
IN UINT16 OptionLen
)
{
- UINT16 Offset;
- UINT8 OptionType;
+ UINT32 Offset;
UINT16 Length;
+ IP6_OPTION_HEADER *OptionHeader;
+
+ if (Option == NULL) {
+ ASSERT (Option != NULL);
+ return FALSE;
+ }
Offset = 0;
- while (Offset < OptionLen) {
- OptionType = *(Option + Offset);
- Length = (UINT16) (*(Option + Offset + 1) * 8);
+ //
+ // RFC 4861 states that Neighbor Discovery packet can contain zero or more
+ // options. Start processing the options if at least Type + Length fields
+ // fit within the input buffer.
+ //
+ while (Offset + sizeof (IP6_OPTION_HEADER) - 1 < OptionLen) {
+ OptionHeader = (IP6_OPTION_HEADER*) (Option + Offset);
+ Length = (UINT16) OptionHeader->Length * 8;
- switch (OptionType) {
+ switch (OptionHeader->Type) {
case Ip6OptionPrefixInfo:
if (Length != 32) {
return FALSE;
}
-
break;
case Ip6OptionMtu:
if (Length != 8) {
return FALSE;
}
-
break;
default:
- //
- // Check the length of Ip6OptionEtherSource, Ip6OptionEtherTarget, and
- // Ip6OptionRedirected here. For unrecognized options, silently ignore
- // and continue processing the message.
- //
+ // RFC 4861 states that Length field cannot be 0.
if (Length == 0) {
return FALSE;
}
+ break;
+ }
+
+ //
+ // Check whether recognized options are within the input buffer's scope.
+ //
+ switch (OptionHeader->Type) {
+ case Ip6OptionEtherSource:
+ case Ip6OptionEtherTarget:
+ case Ip6OptionPrefixInfo:
+ case Ip6OptionRedirected:
+ case Ip6OptionMtu:
+ if (Offset + Length > (UINT32) OptionLen) {
+ return FALSE;
+ }
+ break;
+ default:
+ //
+ // Unrecognized options can be either valid (but unused) or invalid
+ // (garbage in between or right after valid options). Silently ignore.
+ //
break;
}
- Offset = (UINT16) (Offset + Length);
+ //
+ // Advance to the next option.
+ // Length already considers option header's Type + Length.
+ //
+ Offset += Length;
}
return TRUE;
--
2.25.1
>From 19f82aecd83989cde630c1f8eb119713f1574908 Mon Sep 17 00:00:00 2001
From: Maciej Rabeda <maciej.rab...@linux.intel.com>
Date: Wed, 1 Apr 2020 11:43:55 +0200
Subject: [PATCH 3/3] NetworkPkg/Ip6Dxe: Fix ASSERT logic in
Ip6ProcessRouterAdvertise()
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2655
This patch fixes reversed logic of recently added ASSERTs which should
ensure that Ip6IsNDOptionValid() implementation properly reacts to invalid
packets.
Cc: Jiaxin Wu <jiaxin...@intel.com>
Cc: Siyuan Fu <siyuan...@intel.com>
Signed-off-by: Maciej Rabeda <maciej.rab...@linux.intel.com>
Reviewed-by: Siyuan Fu <siyuan...@intel.com>
Tested-by: Laszlo Ersek <ler...@redhat.com>
Fixes: 9c20342eed70ec99ec50cd73cb81804299f05403
(cherry picked from commit 4deef2d865efdc61d1a53ad7bd48f9dd42560b45)
---
NetworkPkg/Ip6Dxe/Ip6Nd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/NetworkPkg/Ip6Dxe/Ip6Nd.c b/NetworkPkg/Ip6Dxe/Ip6Nd.c
index 1254f0fdd921..9e688b2eb728 100644
--- a/NetworkPkg/Ip6Dxe/Ip6Nd.c
+++ b/NetworkPkg/Ip6Dxe/Ip6Nd.c
@@ -2111,7 +2111,7 @@ Ip6ProcessRouterAdvertise (
// Option size validity ensured by Ip6IsNDOptionValid().
//
ASSERT (LinkLayerOption.Length != 0);
- ASSERT (Offset + (UINT32) LinkLayerOption.Length * 8 >= (UINT32)
Head->PayloadLength);
+ ASSERT (Offset + (UINT32) LinkLayerOption.Length * 8 <= (UINT32)
Head->PayloadLength);
ZeroMem (&LinkLayerAddress, sizeof (EFI_MAC_ADDRESS));
CopyMem (&LinkLayerAddress, LinkLayerOption.EtherAddr, 6);
@@ -2164,7 +2164,7 @@ Ip6ProcessRouterAdvertise (
// Option size validity ensured by Ip6IsNDOptionValid().
//
ASSERT (PrefixOption.Length == 4);
- ASSERT (Offset + (UINT32) PrefixOption.Length * 8 >= (UINT32)
Head->PayloadLength);
+ ASSERT (Offset + (UINT32) PrefixOption.Length * 8 <= (UINT32)
Head->PayloadLength);
PrefixOption.ValidLifetime = NTOHL (PrefixOption.ValidLifetime);
PrefixOption.PreferredLifetime = NTOHL (PrefixOption.PreferredLifetime);
@@ -2334,7 +2334,7 @@ Ip6ProcessRouterAdvertise (
// Option size validity ensured by Ip6IsNDOptionValid().
//
ASSERT (MTUOption.Length == 1);
- ASSERT (Offset + (UINT32) MTUOption.Length * 8 >= (UINT32)
Head->PayloadLength);
+ ASSERT (Offset + (UINT32) MTUOption.Length * 8 <= (UINT32)
Head->PayloadLength);
//
// Use IPv6 minimum link MTU 1280 bytes as the maximum packet size in
order
--
2.25.1
