packages/openssl
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix CVE-2024-4741

Browse Source
This commit is contained in:
hzero1996 2024-06-03 16:22:40 +08:00
parent 99f3658802
commit 9794667ec1
4 changed files with 404 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
backport-CVE-2024-4741-Only-free-the-read-buffer.patch Normal file
View File

@ -0,0 +1,71 @@
From b3f0eb0a295f58f16ba43ba99dad70d4ee5c437d Mon Sep 17 00:00:00 2001
From: Watson Ladd <watsonbladd@gmail.com>
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 <tomas@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(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;
}

64
backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-.patch Normal file
View File

@ -0,0 +1,64 @@
From 5d0aba426b076094f74c5910a7e7bf7c0026148b Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
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 <tomas@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(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
@@ -16,6 +16,7 @@
#include <openssl/rand.h>
#include "record_local.h"
#include "../packet_local.h"
+#include "internal/cryptlib.h"
#if defined(OPENSSL_SMALL_FOOTPRINT) || \
!( defined(AESNI_ASM) && ( \
@@ -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

262
backport-CVE-2024-4741-test-Fix-possible-use-after-free.patch Normal file
View File

@ -0,0 +1,262 @@
From 1d8c4ef6a90b87f3b516e9310a0e53bf1425a780 Mon Sep 17 00:00:00 2001
From: c30013472 <chengqguangqiang1@huawei.com>
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 <chengqguangqiang1@huawei.com>
---
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 <openssl/ssl.h>
#include <openssl/bio.h>
#include <openssl/err.h>
-
+#include <openssl/engine.h>
#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 <string.h>
+#include <openssl/engine.h>
#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

8
openssl.spec
View File

@ -2,7 +2,7 @@
Name: openssl
Epoch: 1
Version: 1.1.1m
Release: 31
Release: 32
Summary: Cryptography and SSL/TLS Toolkit
License: OpenSSL and SSLeay
URL: https://www.openssl.org/
@ -79,6 +79,9 @@ Patch68: backport-Add-a-test-for-session-cache-handling.patch
Patch69: backport-Extend-the-multi_resume-test-for-simultaneous-resump.patch
Patch70: backport-Hardening-around-not_resumable-sessions.patch
Patch71: backport-Add-a-test-for-session-cache-overflow.patch
Patch72: backport-CVE-2024-4741-Only-free-the-read-buffer.patch
Patch73: backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-.patch
Patch74: 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}
@ -287,6 +290,9 @@ make test || :
%ldconfig_scriptlets libs
%changelog
* Mon Jun 3 2024 wangcheng <wangcheng156@huawei.com> - 1:1.1.1m-32
- fix CVE-2024-4741
* Wed Apr 17 2024 fuanan <fuanan3@h-partners.com> - 1:1.1.1m-31
- fix CVE-2024-2511