packages/openssl
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
openssl/backport-CVE-2024-4741-test-Fix-possible-use-after-free.patch
hzero1996 9794667ec1 fix CVE-2024-4741
2024-06-04 09:22:37 +08:00

263 lines
7.6 KiB
Diff
Raw Permalink Blame History

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
Reference in New Issue View Git Blame Copy Permalink