From dd15fb080c44e3eaf25c52a7950e0b7e170bec1e Mon Sep 17 00:00:00 2001 From: hzero1996 Date: Mon, 3 Jun 2024 17:02:56 +0800 Subject: [PATCH] fix CVE-2024-4741 --- ...-2024-4741-Only-free-the-read-buffer.patch | 71 +++++ ...t-rlayer.packet-to-NULL-after-we-ve-.patch | 55 ++++ ...741-test-Fix-possible-use-after-free.patch | 262 ++++++++++++++++++ openssl.spec | 8 +- 4 files changed, 395 insertions(+), 1 deletion(-) create mode 100644 backport-CVE-2024-4741-Only-free-the-read-buffer.patch create mode 100644 backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-.patch create mode 100644 backport-CVE-2024-4741-test-Fix-possible-use-after-free.patch diff --git a/backport-CVE-2024-4741-Only-free-the-read-buffer.patch b/backport-CVE-2024-4741-Only-free-the-read-buffer.patch new file mode 100644 index 0000000..a4fc6fa --- /dev/null +++ b/backport-CVE-2024-4741-Only-free-the-read-buffer.patch @@ -0,0 +1,71 @@ +From b3f0eb0a295f58f16ba43ba99dad70d4ee5c437d Mon Sep 17 00:00:00 2001 +From: Watson Ladd +Date: Wed, 24 Apr 2024 11:26:56 +0100 +Subject: [PATCH] Only free the read buffers if we're not using them + +If we're part way through processing a record, or the application has +not released all the records then we should not free our buffer because +they are still needed. + +CVE-2024-4741 + +Reviewed-by: Tomas Mraz +Reviewed-by: Neil Horman +Reviewed-by: Matt Caswell +(Merged from https://github.com/openssl/openssl/pull/24395) + +(cherry picked from commit 704f725b96aa373ee45ecfb23f6abfe8be8d9177) +Reference:https://github.com/openssl/openssl/commit/b3f0eb0a295f58f16ba43ba99dad70d4ee5c437d +Conflict:NA +--- + ssl/record/rec_layer_s3.c | 9 +++++++++ + ssl/record/record.h | 1 + + ssl/ssl_lib.c | 3 +++ + 3 files changed, 13 insertions(+) + +diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c +index 4bcffcc41e364..1569997bea2d3 100644 +--- a/ssl/record/rec_layer_s3.c ++++ b/ssl/record/rec_layer_s3.c +@@ -81,6 +81,15 @@ int RECORD_LAYER_read_pending(const RECORD_LAYER *rl) + return SSL3_BUFFER_get_left(&rl->rbuf) != 0; + } + ++int RECORD_LAYER_data_present(const RECORD_LAYER *rl) ++{ ++ if (rl->rstate == SSL_ST_READ_BODY) ++ return 1; ++ if (RECORD_LAYER_processed_read_pending(rl)) ++ return 1; ++ return 0; ++} ++ + /* Checks if we have decrypted unread record data pending */ + int RECORD_LAYER_processed_read_pending(const RECORD_LAYER *rl) + { +diff --git a/ssl/record/record.h b/ssl/record/record.h +index 234656bf93942..b60f71c8cb23b 100644 +--- a/ssl/record/record.h ++++ b/ssl/record/record.h +@@ -205,6 +205,7 @@ void RECORD_LAYER_release(RECORD_LAYER *rl); + int RECORD_LAYER_read_pending(const RECORD_LAYER *rl); + int RECORD_LAYER_processed_read_pending(const RECORD_LAYER *rl); + int RECORD_LAYER_write_pending(const RECORD_LAYER *rl); ++int RECORD_LAYER_data_present(const RECORD_LAYER *rl); + void RECORD_LAYER_reset_read_sequence(RECORD_LAYER *rl); + void RECORD_LAYER_reset_write_sequence(RECORD_LAYER *rl); + int RECORD_LAYER_is_sslv2_record(RECORD_LAYER *rl); +diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c +index eed649c6fdee9..d14c55ae557bc 100644 +--- a/ssl/ssl_lib.c ++++ b/ssl/ssl_lib.c +@@ -5492,6 +5492,9 @@ int SSL_free_buffers(SSL *ssl) + if (RECORD_LAYER_read_pending(rl) || RECORD_LAYER_write_pending(rl)) + return 0; + ++ if (RECORD_LAYER_data_present(rl)) ++ return 0; ++ + RECORD_LAYER_release(rl); + return 1; + } diff --git a/backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-.patch b/backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-.patch new file mode 100644 index 0000000..034b2d3 --- /dev/null +++ b/backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-.patch @@ -0,0 +1,55 @@ +From 5d0aba426b076094f74c5910a7e7bf7c0026148b Mon Sep 17 00:00:00 2001 +From: Matt Caswell +Date: Wed, 29 May 2024 16:17:48 +0800 +Subject: [PATCH] Set rlayer.packet to NULL after we've finished using it + +In order to ensure we do not have a UAF we reset the rlayer.packet pointer +to NULL after we free it. + +CVE-2024-4741 + +Reviewed-by: Tomas Mraz +Reviewed-by: Neil Horman +(Merged from #24395) + +Reference:https://github.com/openssl/openssl/commit/2d05959073c4bf8803401668b9df85931a08e020 +Conflict:Context Adaptation + +(cherry picked from commit d146349) +--- + ssl/record/rec_layer_s3.c | 6 ++++++ + ssl/record/ssl3_buffer.c | 2 ++ + 2 files changed, 8 insertions(+) + +diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c +index 81d20ad..71b0413 100644 +--- a/ssl/record/rec_layer_s3.c ++++ b/ssl/record/rec_layer_s3.c +@@ -248,6 +248,12 @@ int ssl3_read_n(SSL *s, size_t n, size_t max, int extend, int clearold, + /* ... now we can act as if 'extend' was set */ + } + ++ if (!ossl_assert(s->rlayer.packet != NULL)) { ++ /* does not happen */ ++ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_READ_N, ERR_R_INTERNAL_ERROR); ++ return -1; ++ } ++ + len = s->rlayer.packet_length; + pkt = rb->buf + align; + /* +diff --git a/ssl/record/ssl3_buffer.c b/ssl/record/ssl3_buffer.c +index fa597c2..b8b91d1 100644 +--- a/ssl/record/ssl3_buffer.c ++++ b/ssl/record/ssl3_buffer.c +@@ -179,5 +179,7 @@ int ssl3_release_read_buffer(SSL *s) + b = RECORD_LAYER_get_rbuf(&s->rlayer); + OPENSSL_free(b->buf); + b->buf = NULL; ++ s->rlayer.packet = NULL; ++ s->rlayer.packet_length = 0; + return 1; + } +-- +2.27.0 + diff --git a/backport-CVE-2024-4741-test-Fix-possible-use-after-free.patch b/backport-CVE-2024-4741-test-Fix-possible-use-after-free.patch new file mode 100644 index 0000000..6c805ab --- /dev/null +++ b/backport-CVE-2024-4741-test-Fix-possible-use-after-free.patch @@ -0,0 +1,262 @@ +From 1d8c4ef6a90b87f3b516e9310a0e53bf1425a780 Mon Sep 17 00:00:00 2001 +From: c30013472 +Date: Wed, 29 May 2024 15:25:31 +0800 +Subject: [PATCH] Fix possible use-after-free in SSL_free_buffers + +Related to CVE-2024-4741 + +Reference:https://gitee.com/openeuler/openssl/commit/1d8c4ef6a90b87f3b516e9310a0e53bf1425a780 +Conflict:rec_layer_s3.c,ssl3_buffer.c,ssl_lib.c + +Signed-off-by: c30013472 +--- + test/sslbuffertest.c | 167 +++++++++++++++++++++++++++++++++++++- + test/ssltestlib.c | 25 ++++++ + test/ssltestlib.h | 1 + + 7 files changed, 213 insertions(+), 1 deletion(-) + +diff --git a/test/sslbuffertest.c b/test/sslbuffertest.c +index b5f815f7db..f57db36630 100644 +--- a/test/sslbuffertest.c ++++ b/test/sslbuffertest.c +@@ -12,7 +12,7 @@ + #include + #include + #include +- ++#include + #include "../ssl/packet_local.h" + + #include "ssltestlib.h" +@@ -157,6 +157,166 @@ int global_init(void) + return 1; + } + ++/* ++ * Test that attempting to free the buffers at points where they cannot be freed ++ * works as expected ++ * Test 0: Attempt to free buffers after a full record has been processed, but ++ * the application has only performed a partial read ++ * Test 1: Attempt to free buffers after only a partial record header has been ++ * received ++ * Test 2: Attempt to free buffers after a full record header but no record body ++ * Test 3: Attempt to free buffers after a full record hedaer and partial record ++ * body ++ * Test 4-7: We repeat tests 0-3 but including data from a second pipelined ++ * record ++ */ ++static int test_free_buffers(int test) ++{ ++ int result = 0; ++ SSL *serverssl = NULL, *clientssl = NULL; ++ const char testdata[] = "Test data"; ++ char buf[120]; ++ size_t written, readbytes; ++ int i, pipeline = test > 3; ++ ENGINE *e = NULL; ++ ++ if (pipeline) { ++ e = load_dasync(); ++ if (e == NULL) ++ goto end; ++ test -= 4; ++ } ++ ++ if (!TEST_true(create_ssl_objects(serverctx, clientctx, &serverssl, ++ &clientssl, NULL, NULL))) ++ goto end; ++ ++ if (pipeline) { ++ if (!TEST_true(SSL_set_cipher_list(serverssl, "AES128-SHA")) ++ || !TEST_true(SSL_set_max_proto_version(serverssl, ++ TLS1_2_VERSION)) ++ || !TEST_true(SSL_set_max_pipelines(serverssl, 2))) ++ goto end; ++ } ++ ++ if (!TEST_true(create_ssl_connection(serverssl, clientssl, ++ SSL_ERROR_NONE))) ++ goto end; ++ ++ ++ /* ++ * For the non-pipeline case we write one record. For pipelining we write ++ * two records. ++ */ ++ for (i = 0; i <= pipeline; i++) { ++ if (!TEST_true(SSL_write_ex(clientssl, testdata, strlen(testdata), ++ &written))) ++ goto end; ++ } ++ ++ if (test == 0) { ++ size_t readlen = 1; ++ ++ /* ++ * Deliberately only read the first byte - so the remaining bytes are ++ * still buffered. In the pipelining case we read as far as the first ++ * byte from the second record. ++ */ ++ if (pipeline) ++ readlen += strlen(testdata); ++ ++ if (!TEST_true(SSL_read_ex(serverssl, buf, readlen, &readbytes)) ++ || !TEST_size_t_eq(readlen, readbytes)) ++ goto end; ++ } else { ++ BIO *tmp; ++ size_t partial_len; ++ ++ /* Remove all the data that is pending for read by the server */ ++ tmp = SSL_get_rbio(serverssl); ++ if (!TEST_true(BIO_read_ex(tmp, buf, sizeof(buf), &readbytes)) ++ || !TEST_size_t_lt(readbytes, sizeof(buf)) ++ || !TEST_size_t_gt(readbytes, SSL3_RT_HEADER_LENGTH)) ++ goto end; ++ ++ switch(test) { ++ case 1: ++ partial_len = SSL3_RT_HEADER_LENGTH - 1; ++ break; ++ case 2: ++ partial_len = SSL3_RT_HEADER_LENGTH; ++ break; ++ case 3: ++ partial_len = readbytes - 1; ++ break; ++ default: ++ TEST_error("Invalid test index"); ++ goto end; ++ } ++ ++ if (pipeline) { ++ /* We happen to know the first record is 57 bytes long */ ++ const size_t first_rec_len = 57; ++ ++ if (test != 3) ++ partial_len += first_rec_len; ++ ++ /* ++ * Sanity check. If we got the record len right then this should ++ * never fail. ++ */ ++ if (!TEST_int_eq(buf[first_rec_len], SSL3_RT_APPLICATION_DATA)) ++ goto end; ++ } ++ /* ++ * Put back just the partial record (plus the whole initial record in ++ * the pipelining case) ++ */ ++ if (!TEST_true(BIO_write_ex(tmp, buf, partial_len, &written))) ++ goto end; ++ ++ if (pipeline) { ++ /* ++ * Attempt a read. This should pass but only return data from the ++ * first record. Only a partial record is available for the second ++ * record. ++ */ ++ if (!TEST_true(SSL_read_ex(serverssl, buf, sizeof(buf), ++ &readbytes)) ++ || !TEST_size_t_eq(readbytes, strlen(testdata))) ++ goto end; ++ } else { ++ /* ++ * Attempt a read. This should fail because only a partial record is ++ * available. ++ */ ++ if (!TEST_false(SSL_read_ex(serverssl, buf, sizeof(buf), ++ &readbytes))) ++ goto end; ++ } ++ } ++ ++ /* ++ * Attempting to free the buffers at this point should fail because they are ++ * still in use ++ */ ++ if (!TEST_false(SSL_free_buffers(serverssl))) ++ goto end; ++ ++ result = 1; ++ end: ++ SSL_free(clientssl); ++ SSL_free(serverssl); ++#ifndef OPENSSL_NO_DYNAMIC_ENGINE ++ if (e != NULL) { ++ ENGINE_unregister_ciphers(e); ++ ENGINE_finish(e); ++ ENGINE_free(e); ++ } ++#endif ++ return result; ++} ++ + int setup_tests(void) + { + char *cert, *pkey; +@@ -173,6 +333,11 @@ int setup_tests(void) + } + + ADD_ALL_TESTS(test_func, 9); ++#if !defined(OPENSSL_NO_TLS1_2) && !defined(OPENSSL_NO_DYNAMIC_ENGINE) ++ ADD_ALL_TESTS(test_free_buffers, 8); ++#else ++ ADD_ALL_TESTS(test_free_buffers, 4); ++#endif + return 1; + } + +diff --git a/test/ssltestlib.c b/test/ssltestlib.c +index 422787b0f5..b1da6866c0 100644 +--- a/test/ssltestlib.c ++++ b/test/ssltestlib.c +@@ -9,6 +9,7 @@ + + #include + ++#include + #include "internal/nelem.h" + #include "ssltestlib.h" + #include "testutil.h" +@@ -975,3 +976,27 @@ void shutdown_ssl_connection(SSL *serverssl, SSL *clientssl) + SSL_free(serverssl); + SSL_free(clientssl); + } ++ ++ENGINE *load_dasync(void) ++{ ++#if !defined(OPENSSL_NO_TLS1_2) && !defined(OPENSSL_NO_DYNAMIC_ENGINE) ++ ENGINE *e; ++ ++ if (!TEST_ptr(e = ENGINE_by_id("dasync"))) ++ return NULL; ++ ++ if (!TEST_true(ENGINE_init(e))) { ++ ENGINE_free(e); ++ return NULL; ++ } ++ ++ if (!TEST_true(ENGINE_register_ciphers(e))) { ++ ENGINE_free(e); ++ return NULL; ++ } ++ ++ return e; ++#else ++ return NULL; ++#endif ++} +diff --git a/test/ssltestlib.h b/test/ssltestlib.h +index 8f0a1b5308..38f97a8cbd 100644 +--- a/test/ssltestlib.h ++++ b/test/ssltestlib.h +@@ -54,4 +54,5 @@ typedef struct mempacket_st MEMPACKET; + + DEFINE_STACK_OF(MEMPACKET) + ++ENGINE *load_dasync(void); + #endif /* OSSL_TEST_SSLTESTLIB_H */ +-- +2.27.0 + diff --git a/openssl.spec b/openssl.spec index 36898c0..fa371a4 100644 --- a/openssl.spec +++ b/openssl.spec @@ -2,7 +2,7 @@ Name: openssl Epoch: 1 Version: 1.1.1wa -Release: 6 +Release: 7 Summary: Cryptography and SSL/TLS Toolkit License: OpenSSL and SSLeay URL: https://gitee.com/openeuler/openssl @@ -20,6 +20,9 @@ Patch9: backport-Add-a-test-for-session-cache-handling.patch Patch10: backport-Extend-the-multi_resume-test-for-simultaneous-resump.patch Patch11: backport-Hardening-around-not_resumable-sessions.patch Patch12: backport-Add-a-test-for-session-cache-overflow.patch +Patch13: backport-CVE-2024-4741-Only-free-the-read-buffer.patch +Patch14: backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-.patch +Patch15: backport-CVE-2024-4741-test-Fix-possible-use-after-free.patch BuildRequires: gcc perl make lksctp-tools-devel coreutils util-linux zlib-devel Requires: coreutils %{name}-libs%{?_isa} = %{epoch}:%{version}-%{release} @@ -228,6 +231,9 @@ make test || : %ldconfig_scriptlets libs %changelog +* Mon Jun 3 2024 wangcheng - 1:1.1.1wa-7 +- fix CVE-2024-4741 + * Wed Apr 17 2024 fuanan - 1:1.1.1wa-6 - fix CVE-2024-2511 -- Gitee