Skip to content

Commit 1acab3d

Browse files
[AUTO-CHERRYPICK] Patch CVE-2023-44487 and CVE-2023-35945 in cmake - branch 3.0-dev (#12964)
Co-authored-by: corvus-callidus <108946721+corvus-callidus@users.noreply.github.com>
1 parent d459b0c commit 1acab3d

5 files changed

Lines changed: 613 additions & 6 deletions

File tree

SPECS/cmake/CVE-2023-35945.patch

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
From c9fb4415b76e6cf388c3c7105481b90b1a4c8c29 Mon Sep 17 00:00:00 2001
2+
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
3+
Date: Fri, 14 Jul 2023 20:52:03 +0900
4+
Subject: [PATCH] Fix memory leak
5+
6+
This commit fixes memory leak that happens when PUSH_PROMISE or
7+
HEADERS frame cannot be sent, and nghttp2_on_stream_close_callback
8+
fails with a fatal error. For example, if GOAWAY frame has been
9+
received, a HEADERS frame that opens new stream cannot be sent.
10+
11+
This issue has already been made public via CVE-2023-35945 [1] issued
12+
by envoyproxy/envoy project. During embargo period, the patch to fix
13+
this bug was accidentally submitted to nghttp2/nghttp2 repository [2].
14+
And they decided to disclose CVE early. I was notified just 1.5 hours
15+
before disclosure. I had no time to respond.
16+
17+
PoC described in [1] is quite simple, but I think it is not enough to
18+
trigger this bug. While it is true that receiving GOAWAY prevents a
19+
client from opening new stream, and nghttp2 enters error handling
20+
branch, in order to cause the memory leak,
21+
nghttp2_session_close_stream function must return a fatal error.
22+
nghttp2 defines 2 fatal error codes:
23+
24+
- NGHTTP2_ERR_NOMEM
25+
- NGHTTP2_ERR_CALLBACK_FAILURE
26+
27+
NGHTTP2_ERR_NOMEM, as its name suggests, indicates out of memory. It
28+
is unlikely that a process gets short of memory with this simple PoC
29+
scenario unless application does something memory heavy processing.
30+
31+
NGHTTP2_ERR_CALLBACK_FAILURE is returned from application defined
32+
callback function (nghttp2_on_stream_close_callback, in this case),
33+
which indicates something fatal happened inside a callback, and a
34+
connection must be closed immediately without any further action. As
35+
nghttp2_on_stream_close_error_callback documentation says, any error
36+
code other than 0 or NGHTTP2_ERR_CALLBACK_FAILURE is treated as fatal
37+
error code. More specifically, it is treated as if
38+
NGHTTP2_ERR_CALLBACK_FAILURE is returned. I guess that envoy returns
39+
NGHTTP2_ERR_CALLBACK_FAILURE or other error code which is translated
40+
into NGHTTP2_ERR_CALLBACK_FAILURE.
41+
42+
[1] https://github.com/envoyproxy/envoy/security/advisories/GHSA-jfxv-29pc-x22r
43+
[2] https://github.com/nghttp2/nghttp2/pull/1929
44+
---
45+
lib/nghttp2_session.c | 10 +++++-----
46+
tests/nghttp2_session_test.c | 34 ++++++++++++++++++++++++++++++++++
47+
2 files changed, 39 insertions(+), 5 deletions(-)
48+
49+
diff --git a/Utilities/cmnghttp2/lib/nghttp2_session.c b/lib/nghttp2_session.c
50+
index 9df3d6f3..45392f1a 100644
51+
--- a/Utilities/cmnghttp2/lib/nghttp2_session.c
52+
+++ b/Utilities/cmnghttp2/lib/nghttp2_session.c
53+
@@ -2930,6 +2930,7 @@ static ssize_t nghttp2_session_mem_send_internal(nghttp2_session *session,
54+
if (rv < 0) {
55+
int32_t opened_stream_id = 0;
56+
uint32_t error_code = NGHTTP2_INTERNAL_ERROR;
57+
+ int rv2 = 0;
58+
59+
DEBUGF("send: frame preparation failed with %s\n",
60+
nghttp2_strerror(rv));
61+
@@ -2972,19 +2973,18 @@ static ssize_t nghttp2_session_mem_send_internal(nghttp2_session *session,
62+
}
63+
if (opened_stream_id) {
64+
/* careful not to override rv */
65+
- int rv2;
66+
rv2 = nghttp2_session_close_stream(session, opened_stream_id,
67+
error_code);
68+
-
69+
- if (nghttp2_is_fatal(rv2)) {
70+
- return rv2;
71+
- }
72+
}
73+
74+
nghttp2_outbound_item_free(item, mem);
75+
nghttp2_mem_free(mem, item);
76+
active_outbound_item_reset(aob, mem);
77+
78+
+ if (nghttp2_is_fatal(rv2)) {
79+
+ return rv2;
80+
+ }
81+
+
82+
if (rv == NGHTTP2_ERR_HEADER_COMP) {
83+
/* If header compression error occurred, should terminiate
84+
connection. */
85+
--
86+
2.17.1

0 commit comments

Comments
 (0)