!35 fix CVE-2023-35945
From: @yeah_wang Reviewed-by: @sunsuwan Signed-off-by: @sunsuwan
This commit is contained in:
commit
480099a0b9
143
backport-CVE-2023-35945.patch
Normal file
143
backport-CVE-2023-35945.patch
Normal file
@ -0,0 +1,143 @@
|
|||||||
|
From ce385d3f55a4b76da976b3bdf71fe2deddf315ba Mon Sep 17 00:00:00 2001
|
||||||
|
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
|
||||||
|
Date: Fri, 14 Jul 2023 20:52:03 +0900
|
||||||
|
Subject: [PATCH] Fix memory leak
|
||||||
|
|
||||||
|
This commit fixes memory leak that happens when PUSH_PROMISE or
|
||||||
|
HEADERS frame cannot be sent, and nghttp2_on_stream_close_callback
|
||||||
|
fails with a fatal error. For example, if GOAWAY frame has been
|
||||||
|
received, a HEADERS frame that opens new stream cannot be sent.
|
||||||
|
|
||||||
|
This issue has already been made public via CVE-2023-35945 [1] issued
|
||||||
|
by envoyproxy/envoy project. During embargo period, the patch to fix
|
||||||
|
this bug was accidentally submitted to nghttp2/nghttp2 repository [2].
|
||||||
|
And they decided to disclose CVE early. I was notified just 1.5 hours
|
||||||
|
before disclosure. I had no time to respond.
|
||||||
|
|
||||||
|
PoC described in [1] is quite simple, but I think it is not enough to
|
||||||
|
trigger this bug. While it is true that receiving GOAWAY prevents a
|
||||||
|
client from opening new stream, and nghttp2 enters error handling
|
||||||
|
branch, in order to cause the memory leak,
|
||||||
|
nghttp2_session_close_stream function must return a fatal error.
|
||||||
|
nghttp2 defines 2 fatal error codes:
|
||||||
|
|
||||||
|
- NGHTTP2_ERR_NOMEM
|
||||||
|
- NGHTTP2_ERR_CALLBACK_FAILURE
|
||||||
|
|
||||||
|
NGHTTP2_ERR_NOMEM, as its name suggests, indicates out of memory. It
|
||||||
|
is unlikely that a process gets short of memory with this simple PoC
|
||||||
|
scenario unless application does something memory heavy processing.
|
||||||
|
|
||||||
|
NGHTTP2_ERR_CALLBACK_FAILURE is returned from application defined
|
||||||
|
callback function (nghttp2_on_stream_close_callback, in this case),
|
||||||
|
which indicates something fatal happened inside a callback, and a
|
||||||
|
connection must be closed immediately without any further action. As
|
||||||
|
nghttp2_on_stream_close_error_callback documentation says, any error
|
||||||
|
code other than 0 or NGHTTP2_ERR_CALLBACK_FAILURE is treated as fatal
|
||||||
|
error code. More specifically, it is treated as if
|
||||||
|
NGHTTP2_ERR_CALLBACK_FAILURE is returned. I guess that envoy returns
|
||||||
|
NGHTTP2_ERR_CALLBACK_FAILURE or other error code which is translated
|
||||||
|
into NGHTTP2_ERR_CALLBACK_FAILURE.
|
||||||
|
|
||||||
|
[1] https://github.com/envoyproxy/envoy/security/advisories/GHSA-jfxv-29pc-x22r
|
||||||
|
[2] https://github.com/nghttp2/nghttp2/pull/1929
|
||||||
|
---
|
||||||
|
lib/nghttp2_session.c | 10 +++++-----
|
||||||
|
tests/nghttp2_session_test.c | 34 ++++++++++++++++++++++++++++++++++
|
||||||
|
2 files changed, 39 insertions(+), 5 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c
|
||||||
|
index 7509ceb5c6..71858a39e0 100644
|
||||||
|
--- a/lib/nghttp2_session.c
|
||||||
|
+++ b/lib/nghttp2_session.c
|
||||||
|
@@ -3296,6 +3296,7 @@ static ssize_t nghttp2_session_mem_send_internal(nghttp2_session *session,
|
||||||
|
if (rv < 0) {
|
||||||
|
int32_t opened_stream_id = 0;
|
||||||
|
uint32_t error_code = NGHTTP2_INTERNAL_ERROR;
|
||||||
|
+ int rv2 = 0;
|
||||||
|
|
||||||
|
DEBUGF("send: frame preparation failed with %s\n",
|
||||||
|
nghttp2_strerror(rv));
|
||||||
|
@@ -3338,19 +3339,18 @@ static ssize_t nghttp2_session_mem_send_internal(nghttp2_session *session,
|
||||||
|
}
|
||||||
|
if (opened_stream_id) {
|
||||||
|
/* careful not to override rv */
|
||||||
|
- int rv2;
|
||||||
|
rv2 = nghttp2_session_close_stream(session, opened_stream_id,
|
||||||
|
error_code);
|
||||||
|
-
|
||||||
|
- if (nghttp2_is_fatal(rv2)) {
|
||||||
|
- return rv2;
|
||||||
|
- }
|
||||||
|
}
|
||||||
|
|
||||||
|
nghttp2_outbound_item_free(item, mem);
|
||||||
|
nghttp2_mem_free(mem, item);
|
||||||
|
active_outbound_item_reset(aob, mem);
|
||||||
|
|
||||||
|
+ if (nghttp2_is_fatal(rv2)) {
|
||||||
|
+ return rv2;
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
if (rv == NGHTTP2_ERR_HEADER_COMP) {
|
||||||
|
/* If header compression error occurred, should terminiate
|
||||||
|
connection. */
|
||||||
|
diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c
|
||||||
|
index b55ff53443..74352426c1 100644
|
||||||
|
--- a/tests/nghttp2_session_test.c
|
||||||
|
+++ b/tests/nghttp2_session_test.c
|
||||||
|
@@ -584,6 +584,15 @@ static int on_stream_close_callback(nghttp2_session *session, int32_t stream_id,
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
+static int fatal_error_on_stream_close_callback(nghttp2_session *session,
|
||||||
|
+ int32_t stream_id,
|
||||||
|
+ uint32_t error_code,
|
||||||
|
+ void *user_data) {
|
||||||
|
+ on_stream_close_callback(session, stream_id, error_code, user_data);
|
||||||
|
+
|
||||||
|
+ return NGHTTP2_ERR_CALLBACK_FAILURE;
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
static ssize_t pack_extension_callback(nghttp2_session *session, uint8_t *buf,
|
||||||
|
size_t len, const nghttp2_frame *frame,
|
||||||
|
void *user_data) {
|
||||||
|
@@ -4296,6 +4305,8 @@ void test_nghttp2_session_on_goaway_received(void) {
|
||||||
|
nghttp2_frame frame;
|
||||||
|
int i;
|
||||||
|
nghttp2_mem *mem;
|
||||||
|
+ const uint8_t *data;
|
||||||
|
+ ssize_t datalen;
|
||||||
|
|
||||||
|
mem = nghttp2_mem_default();
|
||||||
|
user_data.frame_recv_cb_called = 0;
|
||||||
|
@@ -4337,6 +4348,29 @@ void test_nghttp2_session_on_goaway_received(void) {
|
||||||
|
|
||||||
|
nghttp2_frame_goaway_free(&frame.goaway, mem);
|
||||||
|
nghttp2_session_del(session);
|
||||||
|
+
|
||||||
|
+ /* Make sure that no memory leak when stream_close callback fails
|
||||||
|
+ with a fatal error */
|
||||||
|
+ memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
|
||||||
|
+ callbacks.on_stream_close_callback = fatal_error_on_stream_close_callback;
|
||||||
|
+
|
||||||
|
+ memset(&user_data, 0, sizeof(user_data));
|
||||||
|
+
|
||||||
|
+ nghttp2_session_client_new(&session, &callbacks, &user_data);
|
||||||
|
+
|
||||||
|
+ nghttp2_frame_goaway_init(&frame.goaway, 0, NGHTTP2_NO_ERROR, NULL, 0);
|
||||||
|
+
|
||||||
|
+ CU_ASSERT(0 == nghttp2_session_on_goaway_received(session, &frame));
|
||||||
|
+
|
||||||
|
+ nghttp2_submit_request(session, NULL, reqnv, ARRLEN(reqnv), NULL, NULL);
|
||||||
|
+
|
||||||
|
+ datalen = nghttp2_session_mem_send(session, &data);
|
||||||
|
+
|
||||||
|
+ CU_ASSERT(NGHTTP2_ERR_CALLBACK_FAILURE == datalen);
|
||||||
|
+ CU_ASSERT(1 == user_data.stream_close_cb_called);
|
||||||
|
+
|
||||||
|
+ nghttp2_frame_goaway_free(&frame.goaway, mem);
|
||||||
|
+ nghttp2_session_del(session);
|
||||||
|
}
|
||||||
|
|
||||||
|
void test_nghttp2_session_on_window_update_received(void) {
|
||||||
10
nghttp2.spec
10
nghttp2.spec
@ -1,11 +1,13 @@
|
|||||||
Name: nghttp2
|
Name: nghttp2
|
||||||
Version: 1.52.0
|
Version: 1.52.0
|
||||||
Release: 1
|
Release: 2
|
||||||
Summary: Contains the HTTP/2 client, server and proxy programs.
|
Summary: Contains the HTTP/2 client, server and proxy programs.
|
||||||
License: MIT
|
License: MIT
|
||||||
URL: https://nghttp2.org/
|
URL: https://nghttp2.org/
|
||||||
Source0: https://github.com/nghttp2/nghttp2/releases/download/v%{version}/%{name}-%{version}.tar.xz
|
Source0: https://github.com/nghttp2/nghttp2/releases/download/v%{version}/%{name}-%{version}.tar.xz
|
||||||
|
|
||||||
|
Patch0: backport-CVE-2023-35945.patch
|
||||||
|
|
||||||
BuildRequires: CUnit-devel c-ares-devel gcc-c++ libev-devel openssl-devel automake
|
BuildRequires: CUnit-devel c-ares-devel gcc-c++ libev-devel openssl-devel automake
|
||||||
BuildRequires: python3-devel systemd-devel zlib-devel make libxml2-devel
|
BuildRequires: python3-devel systemd-devel zlib-devel make libxml2-devel
|
||||||
|
|
||||||
@ -89,6 +91,12 @@ export "LD_LIBRARY_PATH=$RPM_BUILD_ROOT%{_libdir}:$LD_LIBRARY_PATH"
|
|||||||
%{_mandir}/man1/*
|
%{_mandir}/man1/*
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Sat Jul 15 2023 wangye <wangye91@h-partners.com> - 1.52.0-2
|
||||||
|
- Type:cve
|
||||||
|
- ID:CVE-2023-35945
|
||||||
|
- SUG:NA
|
||||||
|
- DESC:fix CVE-2023-35945
|
||||||
|
|
||||||
* Mon Mar 06 2023 xinghe <xinghe2@h-partners.com> - 1.52.0-1
|
* Mon Mar 06 2023 xinghe <xinghe2@h-partners.com> - 1.52.0-1
|
||||||
- Type:requirements
|
- Type:requirements
|
||||||
- Id:NA
|
- Id:NA
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user