sysmocom
/
asterisk
11
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bundled_pjproject: Backport security fixes from pjproject 2.13.1 

Merge-pull-request-from-GHSA-9pfh-r8x4-w26w.patch
Merge-pull-request-from-GHSA-cxwq-5g9x-x7fr.patch
Locking-fix-so-that-SSL_shutdown-and-SSL_write-are-n.patch
Don-t-call-SSL_shutdown-when-receiving-SSL_ERROR_SYS.patch

Resolves: #188
This commit is contained in:
George Joseph 2023-07-05 07:35:28 -06:00 committed by asterisk-org-access-app[bot]
parent 6df35a1a6f
commit 05b45ca303
4 changed files with 573 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

203
third-party/pjproject/patches/0300-Merge-pull-request-from-GHSA-9pfh-r8x4-w26w.patch vendored Normal file
View File

@ -0,0 +1,203 @@
From 3ba8f3c0188fa05bb62d8bc9176ca7c7db79f8c0 Mon Sep 17 00:00:00 2001
From: Nanang Izzuddin <nanang@teluu.com>
Date: Tue, 20 Dec 2022 11:39:12 +0700
Subject: [PATCH 300/303] Merge pull request from GHSA-9pfh-r8x4-w26w
* Fix buffer overread in STUN message decoder
* Updates based on comments
---
pjnath/include/pjnath/stun_msg.h | 4 ++++
pjnath/src/pjnath/stun_msg.c | 32 ++++++++++++++++++++------------
2 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/pjnath/include/pjnath/stun_msg.h b/pjnath/include/pjnath/stun_msg.h
index 6b5fc0f21..e8f52db3c 100644
--- a/pjnath/include/pjnath/stun_msg.h
+++ b/pjnath/include/pjnath/stun_msg.h
@@ -436,20 +436,21 @@ typedef enum pj_stun_status
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Transaction ID
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
\endverbatim
*/
+#pragma pack(1)
typedef struct pj_stun_msg_hdr
{
/**
* STUN message type, which the first two bits must be zeroes.
*/
pj_uint16_t type;
/**
* The message length is the size, in bytes, of the message not
* including the 20 byte STUN header.
@@ -467,53 +468,56 @@ typedef struct pj_stun_msg_hdr
* The transaction ID is a 96 bit identifier. STUN transactions are
* identified by their unique 96-bit transaction ID. For request/
* response transactions, the transaction ID is chosen by the STUN
* client and MUST be unique for each new STUN transaction generated by
* that STUN client. The transaction ID MUST be uniformly and randomly
* distributed between 0 and 2**96 - 1.
*/
pj_uint8_t tsx_id[12];
} pj_stun_msg_hdr;
+#pragma pack()
/**
* This structre describes STUN attribute header. Each attribute is
* TLV encoded, with a 16 bit type, 16 bit length, and variable value.
* Each STUN attribute ends on a 32 bit boundary:
*
* \verbatim
0 1 2 3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Type | Length |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
\endverbatim
*/
+#pragma pack(1)
typedef struct pj_stun_attr_hdr
{
/**
* STUN attribute type.
*/
pj_uint16_t type;
/**
* The Length refers to the length of the actual useful content of the
* Value portion of the attribute, measured in bytes. The value
* in the Length field refers to the length of the Value part of the
* attribute prior to padding - i.e., the useful content.
*/
pj_uint16_t length;
} pj_stun_attr_hdr;
+#pragma pack()
/**
* This structure describes STUN generic IP address attribute, used for
* example to represent STUN MAPPED-ADDRESS attribute.
*
* The generic IP address attribute indicates the transport address.
* It consists of an eight bit address family, and a sixteen bit port,
* followed by a fixed length value representing the IP address. If the
* address family is IPv4, the address is 32 bits, in network byte
diff --git a/pjnath/src/pjnath/stun_msg.c b/pjnath/src/pjnath/stun_msg.c
index bd83351e6..fd15230bc 100644
--- a/pjnath/src/pjnath/stun_msg.c
+++ b/pjnath/src/pjnath/stun_msg.c
@@ -739,22 +739,22 @@ PJ_DEF(int) pj_stun_set_padding_char(int chr)
int old_pad = padding_char;
padding_char = chr;
return old_pad;
}
//////////////////////////////////////////////////////////////////////////////
#define INIT_ATTR(a,t,l) (a)->hdr.type=(pj_uint16_t)(t), \
- (a)->hdr.length=(pj_uint16_t)(l)
-#define ATTR_HDR_LEN 4
+ (a)->hdr.length=(pj_uint16_t)(l)
+#define ATTR_HDR_LEN sizeof(pj_stun_attr_hdr)
static pj_uint16_t GETVAL16H(const pj_uint8_t *buf, unsigned pos)
{
return (pj_uint16_t) ((buf[pos + 0] << 8) | \
(buf[pos + 1] << 0));
}
/*unused PJ_INLINE(pj_uint16_t) GETVAL16N(const pj_uint8_t *buf, unsigned pos)
{
return pj_htons(GETVAL16H(buf,pos));
@@ -2318,56 +2318,64 @@ PJ_DEF(pj_status_t) pj_stun_msg_decode(pj_pool_t *pool,
PJ_ASSERT_RETURN(pool && pdu && pdu_len && p_msg, PJ_EINVAL);
PJ_ASSERT_RETURN(sizeof(pj_stun_msg_hdr) == 20, PJ_EBUG);
if (p_parsed_len)
*p_parsed_len = 0;
if (p_response)
*p_response = NULL;
/* Check if this is a STUN message, if necessary */
if (options & PJ_STUN_CHECK_PACKET) {
- status = pj_stun_msg_check(pdu, pdu_len, options);
- if (status != PJ_SUCCESS)
- return status;
+ status = pj_stun_msg_check(pdu, pdu_len, options);
+ if (status != PJ_SUCCESS)
+ return status;
+ } else {
+ /* For safety, verify packet length at least */
+ pj_uint32_t msg_len = GETVAL16H(pdu, 2) + 20;
+ if (msg_len > pdu_len ||
+ ((options & PJ_STUN_IS_DATAGRAM) && msg_len != pdu_len))
+ {
+ return PJNATH_EINSTUNMSGLEN;
+ }
}
/* Create the message, copy the header, and convert to host byte order */
msg = PJ_POOL_ZALLOC_T(pool, pj_stun_msg);
pj_memcpy(&msg->hdr, pdu, sizeof(pj_stun_msg_hdr));
msg->hdr.type = pj_ntohs(msg->hdr.type);
msg->hdr.length = pj_ntohs(msg->hdr.length);
msg->hdr.magic = pj_ntohl(msg->hdr.magic);
pdu += sizeof(pj_stun_msg_hdr);
/* pdu_len -= sizeof(pj_stun_msg_hdr); */
pdu_len = msg->hdr.length;
/* No need to create response if this is not a request */
if (!PJ_STUN_IS_REQUEST(msg->hdr.type))
p_response = NULL;
/* Parse attributes */
- while (pdu_len >= 4) {
- unsigned attr_type, attr_val_len;
- const struct attr_desc *adesc;
+ while (pdu_len >= ATTR_HDR_LEN) {
+ unsigned attr_type, attr_val_len;
+ const struct attr_desc *adesc;
/* Get attribute type and length. If length is not aligned
* to 4 bytes boundary, add padding.
*/
attr_type = GETVAL16H(pdu, 0);
attr_val_len = GETVAL16H(pdu, 2);
attr_val_len = (attr_val_len + 3) & (~3);
- /* Check length */
- if (pdu_len < attr_val_len) {
- pj_str_t err_msg;
- char err_msg_buf[80];
+ /* Check length */
+ if (pdu_len < attr_val_len + ATTR_HDR_LEN) {
+ pj_str_t err_msg;
+ char err_msg_buf[80];
err_msg.ptr = err_msg_buf;
err_msg.slen = pj_ansi_snprintf(err_msg_buf, sizeof(err_msg_buf),
"Attribute %s has invalid length",
pj_stun_get_attr_name(attr_type));
PJ_LOG(4,(THIS_FILE, "Error decoding message: %.*s",
(int)err_msg.slen, err_msg.ptr));
if (p_response) {
--
2.41.0

81
third-party/pjproject/patches/0301-Merge-pull-request-from-GHSA-cxwq-5g9x-x7fr.patch vendored Normal file
View File

@ -0,0 +1,81 @@
From 02d2273f085943b7d8daf7814d9b316216cae26b Mon Sep 17 00:00:00 2001
From: sauwming <ming@teluu.com>
Date: Fri, 23 Dec 2022 15:05:28 +0800
Subject: [PATCH 301/303] Merge pull request from GHSA-cxwq-5g9x-x7fr
* Fixed heap buffer overflow when parsing STUN errcode attribute
* Also fixed uint parsing
---
pjnath/src/pjnath/stun_msg.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/pjnath/src/pjnath/stun_msg.c b/pjnath/src/pjnath/stun_msg.c
index fd15230bc..d3aaae5bf 100644
--- a/pjnath/src/pjnath/stun_msg.c
+++ b/pjnath/src/pjnath/stun_msg.c
@@ -1432,26 +1432,26 @@ static pj_status_t decode_uint_attr(pj_pool_t *pool,
void **p_attr)
{
pj_stun_uint_attr *attr;
PJ_UNUSED_ARG(msghdr);
/* Create the attribute */
attr = PJ_POOL_ZALLOC_T(pool, pj_stun_uint_attr);
GETATTRHDR(buf, &attr->hdr);
- attr->value = GETVAL32H(buf, 4);
-
/* Check that the attribute length is valid */
if (attr->hdr.length != 4)
return PJNATH_ESTUNINATTRLEN;
+ attr->value = GETVAL32H(buf, 4);
+
/* Done */
*p_attr = attr;
return PJ_SUCCESS;
}
static pj_status_t encode_uint_attr(const void *a, pj_uint8_t *buf,
unsigned len,
const pj_stun_msg_hdr *msghdr,
@@ -1751,28 +1751,29 @@ static pj_status_t decode_errcode_attr(pj_pool_t *pool,
{
pj_stun_errcode_attr *attr;
pj_str_t value;
PJ_UNUSED_ARG(msghdr);
/* Create the attribute */
attr = PJ_POOL_ZALLOC_T(pool, pj_stun_errcode_attr);
GETATTRHDR(buf, &attr->hdr);
+ /* Check that the attribute length is valid */
+ if (attr->hdr.length < 4)
+ return PJNATH_ESTUNINATTRLEN;
+
attr->err_code = buf[6] * 100 + buf[7];
/* Get pointer to the string in the message */
value.ptr = ((char*)buf + ATTR_HDR_LEN + 4);
value.slen = attr->hdr.length - 4;
- /* Make sure the length is never negative */
- if (value.slen < 0)
- value.slen = 0;
/* Copy the string to the attribute */
pj_strdup(pool, &attr->reason, &value);
/* Done */
*p_attr = attr;
return PJ_SUCCESS;
}
--
2.41.0

166
third-party/pjproject/patches/0302-Locking-fix-so-that-SSL_shutdown-and-SSL_write-are-n.patch vendored Normal file
View File

@ -0,0 +1,166 @@
From 0a3af5f1a0f64fd30f35338b8328391283d88ecb Mon Sep 17 00:00:00 2001
From: Matthew Fredrickson <mfredrickson@fluentstream.com>
Date: Tue, 30 May 2023 04:33:05 -0500
Subject: [PATCH 302/303] Locking fix so that SSL_shutdown and SSL_write are
not called at same time (#3583)
---
pjlib/src/pj/ssl_sock_ossl.c | 82 ++++++++++++++++++++++--------------
1 file changed, 51 insertions(+), 31 deletions(-)
diff --git a/pjlib/src/pj/ssl_sock_ossl.c b/pjlib/src/pj/ssl_sock_ossl.c
index ed441e3e2..5c8e67b76 100644
--- a/pjlib/src/pj/ssl_sock_ossl.c
+++ b/pjlib/src/pj/ssl_sock_ossl.c
@@ -1627,44 +1627,58 @@ static void ssl_destroy(pj_ssl_sock_t *ssock)
/* Potentially shutdown OpenSSL library if this is the last
* context exists.
*/
shutdown_openssl();
}
/* Reset SSL socket state */
static void ssl_reset_sock_state(pj_ssl_sock_t *ssock)
{
+ int post_unlock_flush_circ_buf = 0;
+
ossl_sock_t *ossock = (ossl_sock_t *)ssock;
+ /* Must lock around SSL calls, particularly SSL_shutdown
+ * as it can modify the write BIOs and destructively
+ * interfere with any ssl_write() calls in progress
+ * above in a multithreaded environment */
+ pj_lock_acquire(ssock->write_mutex);
+
/* Detach from SSL instance */
if (ossock->ossl_ssl) {
SSL_set_ex_data(ossock->ossl_ssl, sslsock_idx, NULL);
}
/**
* Avoid calling SSL_shutdown() if handshake wasn't completed.
* OpenSSL 1.0.2f complains if SSL_shutdown() is called during an
* SSL handshake, while previous versions always return 0.
*/
if (ossock->ossl_ssl && SSL_in_init(ossock->ossl_ssl) == 0) {
- int ret = SSL_shutdown(ossock->ossl_ssl);
- if (ret == 0) {
- /* Flush data to send close notify. */
- flush_circ_buf_output(ssock, &ssock->shutdown_op_key, 0, 0);
- }
+ int ret = SSL_shutdown(ossock->ossl_ssl);
+ if (ret == 0) {
+ /* SSL_shutdown will potentially trigger a bunch of
+ * data to dump to the socket */
+ post_unlock_flush_circ_buf = 1;
+ }
}
- pj_lock_acquire(ssock->write_mutex);
ssock->ssl_state = SSL_STATE_NULL;
+
pj_lock_release(ssock->write_mutex);
+ if (post_unlock_flush_circ_buf) {
+ /* Flush data to send close notify. */
+ flush_circ_buf_output(ssock, &ssock->shutdown_op_key, 0, 0);
+ }
+
ssl_close_sockets(ssock);
/* Upon error, OpenSSL may leave any error description in the thread
* error queue, which sometime may cause next call to SSL API returning
* false error alarm, e.g: in Linux, SSL_CTX_use_certificate_chain_file()
* returning false error after a handshake error (in different SSL_CTX!).
* For now, just clear thread error queue here.
*/
ERR_clear_error();
}
@@ -2330,52 +2344,58 @@ static pj_status_t ssl_read(pj_ssl_sock_t *ssock, void *data, int *size)
{
ossl_sock_t *ossock = (ossl_sock_t *)ssock;
int size_ = *size;
int len = size_;
/* SSL_read() may write some data to write buffer when re-negotiation
* is on progress, so let's protect it with write mutex.
*/
pj_lock_acquire(ssock->write_mutex);
*size = size_ = SSL_read(ossock->ossl_ssl, data, size_);
- pj_lock_release(ssock->write_mutex);
if (size_ <= 0) {
pj_status_t status;
int err = SSL_get_error(ossock->ossl_ssl, size_);
- /* SSL might just return SSL_ERROR_WANT_READ in
- * re-negotiation.
- */
- if (err != SSL_ERROR_NONE && err != SSL_ERROR_WANT_READ) {
- if (err == SSL_ERROR_SYSCALL && size_ == -1 &&
- ERR_peek_error() == 0 && errno == 0)
- {
- status = STATUS_FROM_SSL_ERR2("Read", ssock, size_,
- err, len);
- PJ_LOG(4,("SSL", "SSL_read() = -1, with "
- "SSL_ERROR_SYSCALL, no SSL error, "
- "and errno = 0 - skip BIO error"));
- /* Ignore these errors */
- } else {
- /* Reset SSL socket state, then return PJ_FALSE */
- status = STATUS_FROM_SSL_ERR2("Read", ssock, size_,
- err, len);
- ssl_reset_sock_state(ssock);
- return status;
- }
- }
-
- /* Need renegotiation */
- return PJ_EEOF;
+ /* SSL might just return SSL_ERROR_WANT_READ in
+ * re-negotiation.
+ */
+ if (err != SSL_ERROR_NONE && err != SSL_ERROR_WANT_READ) {
+ if (err == SSL_ERROR_SYSCALL && size_ == -1 &&
+ ERR_peek_error() == 0 && errno == 0)
+ {
+ status = STATUS_FROM_SSL_ERR2("Read", ssock, size_,
+ err, len);
+ PJ_LOG(4,("SSL", "SSL_read() = -1, with "
+ "SSL_ERROR_SYSCALL, no SSL error, "
+ "and errno = 0 - skip BIO error"));
+ /* Ignore these errors */
+ } else {
+ /* Reset SSL socket state, then return PJ_FALSE */
+ status = STATUS_FROM_SSL_ERR2("Read", ssock, size_,
+ err, len);
+ pj_lock_release(ssock->write_mutex);
+ /* Unfortunately we can't hold the lock here to reset all the state.
+ * We probably should though.
+ */
+ ssl_reset_sock_state(ssock);
+ return status;
+ }
+ }
+
+ pj_lock_release(ssock->write_mutex);
+ /* Need renegotiation */
+ return PJ_EEOF;
}
+ pj_lock_release(ssock->write_mutex);
+
return PJ_SUCCESS;
}
/* Write plain data to SSL and flush write BIO. */
static pj_status_t ssl_write(pj_ssl_sock_t *ssock, const void *data,
pj_ssize_t size, int *nwritten)
{
ossl_sock_t *ossock = (ossl_sock_t *)ssock;
pj_status_t status = PJ_SUCCESS;
--
2.41.0

123
third-party/pjproject/patches/0303-Don-t-call-SSL_shutdown-when-receiving-SSL_ERROR_SYS.patch vendored Normal file
View File

@ -0,0 +1,123 @@
From 0f7267f220be79e21cf9f96efa01929285e9aa55 Mon Sep 17 00:00:00 2001
From: Riza Sulistyo <trengginas@users.noreply.github.com>
Date: Wed, 5 Jul 2023 10:38:21 +0700
Subject: [PATCH 303/303] Don't call SSL_shutdown() when receiving
SSL_ERROR_SYSCALL or SSL_ERROR_SSL (#3577)
---
pjlib/src/pj/ssl_sock_imp_common.c | 1 +
pjlib/src/pj/ssl_sock_imp_common.h | 13 +++++++------
pjlib/src/pj/ssl_sock_ossl.c | 17 ++++++++++++-----
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/pjlib/src/pj/ssl_sock_imp_common.c b/pjlib/src/pj/ssl_sock_imp_common.c
index ae2f1136e..c825676c3 100644
--- a/pjlib/src/pj/ssl_sock_imp_common.c
+++ b/pjlib/src/pj/ssl_sock_imp_common.c
@@ -237,20 +237,21 @@ static void ssl_close_sockets(pj_ssl_sock_t *ssock)
#endif
/* When handshake completed:
* - notify application
* - if handshake failed, reset SSL state
* - return PJ_FALSE when SSL socket instance is destroyed by application.
*/
static pj_bool_t on_handshake_complete(pj_ssl_sock_t *ssock,
pj_status_t status)
{
+ ssock->handshake_status = status;
/* Cancel handshake timer */
if (ssock->timer.id == TIMER_HANDSHAKE_TIMEOUT) {
pj_timer_heap_cancel(ssock->param.timer_heap, &ssock->timer);
ssock->timer.id = TIMER_NONE;
}
/* Update certificates info on successful handshake */
if (status == PJ_SUCCESS)
ssl_update_certs_info(ssock);
diff --git a/pjlib/src/pj/ssl_sock_imp_common.h b/pjlib/src/pj/ssl_sock_imp_common.h
index cba28dbd3..8a63faa90 100644
--- a/pjlib/src/pj/ssl_sock_imp_common.h
+++ b/pjlib/src/pj/ssl_sock_imp_common.h
@@ -99,26 +99,27 @@ struct pj_ssl_sock_t
* information allocation. Don't use for
* other purposes. */
pj_ssl_sock_t *parent;
pj_ssl_sock_param param;
pj_ssl_sock_param newsock_param;
pj_ssl_cert_t *cert;
pj_ssl_cert_info local_cert_info;
pj_ssl_cert_info remote_cert_info;
- pj_bool_t is_server;
- enum ssl_state ssl_state;
- pj_ioqueue_op_key_t handshake_op_key;
- pj_ioqueue_op_key_t shutdown_op_key;
- pj_timer_entry timer;
- pj_status_t verify_status;
+ pj_bool_t is_server;
+ enum ssl_state ssl_state;
+ pj_ioqueue_op_key_t handshake_op_key;
+ pj_ioqueue_op_key_t shutdown_op_key;
+ pj_timer_entry timer;
+ pj_status_t verify_status;
+ pj_status_t handshake_status;
pj_bool_t is_closing;
unsigned long last_err;
pj_sock_t sock;
pj_activesock_t *asock;
pj_sockaddr local_addr;
pj_sockaddr rem_addr;
int addr_len;
diff --git a/pjlib/src/pj/ssl_sock_ossl.c b/pjlib/src/pj/ssl_sock_ossl.c
index 5c8e67b76..8a717e362 100644
--- a/pjlib/src/pj/ssl_sock_ossl.c
+++ b/pjlib/src/pj/ssl_sock_ossl.c
@@ -1646,27 +1646,34 @@ static void ssl_reset_sock_state(pj_ssl_sock_t *ssock)
/* Detach from SSL instance */
if (ossock->ossl_ssl) {
SSL_set_ex_data(ossock->ossl_ssl, sslsock_idx, NULL);
}
/**
* Avoid calling SSL_shutdown() if handshake wasn't completed.
* OpenSSL 1.0.2f complains if SSL_shutdown() is called during an
* SSL handshake, while previous versions always return 0.
+ * Call SSL_shutdown() when there is a timeout handshake failure or
+ * the last error is not SSL_ERROR_SYSCALL and not SSL_ERROR_SSL.
*/
if (ossock->ossl_ssl && SSL_in_init(ossock->ossl_ssl) == 0) {
- int ret = SSL_shutdown(ossock->ossl_ssl);
- if (ret == 0) {
- /* SSL_shutdown will potentially trigger a bunch of
- * data to dump to the socket */
- post_unlock_flush_circ_buf = 1;
+ if (ssock->handshake_status == PJ_ETIMEDOUT ||
+ (ssock->last_err != SSL_ERROR_SYSCALL &&
+ ssock->last_err != SSL_ERROR_SSL))
+ {
+ int ret = SSL_shutdown(ossock->ossl_ssl);
+ if (ret == 0) {
+ /* SSL_shutdown will potentially trigger a bunch of
+ * data to dump to the socket */
+ post_unlock_flush_circ_buf = 1;
+ }
}
}
ssock->ssl_state = SSL_STATE_NULL;
pj_lock_release(ssock->write_mutex);
if (post_unlock_flush_circ_buf) {
/* Flush data to send close notify. */
flush_circ_buf_output(ssock, &ssock->shutdown_op_key, 0, 0);
--
2.41.0