From: Alexandr Nedvedicky <sashan@openssl.org>
Date: Mon, 23 Dec 2024 16:03:32 +0000 (+0100)
Subject: Fix read out of buffer bounds when dealing with BIO_ADDR
X-Git-Tag: openssl-3.5.0-alpha1~66
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=395a83a617a09c1ae02e8040386f9acb356d13c1;p=thirdparty%2Fopenssl.git

Fix read out of buffer bounds when dealing with BIO_ADDR

This issue was discoevered while I was testing SSL_new_from_listener()
using a newly created unit test. It has turned out the QUIC stack
at few places contain pattern as follows:
	foo(QUIC_WHATEVER *q, BIO_ADDR *a)
	{
	   q->a = *a;
	}

The problem is that derefencning a that way is risky. If the address `a`
comes from BIO_lookup_ex() it may actually be shorter than sizeof(BIO_ADDR).
Using BIO_ADDR_copy() is the right thing to do here.

Fixes #26241

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26252)
---

diff --git a/crypto/bio/bio_addr.c b/crypto/bio/bio_addr.c
index 568c0b4a1f8..f7d140cbd94 100644
--- a/crypto/bio/bio_addr.c
+++ b/crypto/bio/bio_addr.c
@@ -104,6 +104,7 @@ void BIO_ADDR_clear(BIO_ADDR *ap)
  */
 int BIO_ADDR_make(BIO_ADDR *ap, const struct sockaddr *sa)
 {
+    memset(ap, 0, sizeof(BIO_ADDR));
     if (sa->sa_family == AF_INET) {
         memcpy(&(ap->s_in), sa, sizeof(struct sockaddr_in));
         return 1;
diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index 169d96a8ded..72424b0b7ca 100644
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -513,8 +513,7 @@ int ossl_quic_channel_get_peer_addr(QUIC_CHANNEL *ch, BIO_ADDR *peer_addr)
     if (!ch->addressed_mode)
         return 0;
 
-    *peer_addr = ch->cur_peer_addr;
-    return 1;
+    return BIO_ADDR_copy(peer_addr, &ch->cur_peer_addr);
 }
 
 int ossl_quic_channel_set_peer_addr(QUIC_CHANNEL *ch, const BIO_ADDR *peer_addr)
@@ -528,8 +527,12 @@ int ossl_quic_channel_set_peer_addr(QUIC_CHANNEL *ch, const BIO_ADDR *peer_addr)
         return 1;
     }
 
-    ch->cur_peer_addr   = *peer_addr;
-    ch->addressed_mode  = 1;
+    if (!BIO_ADDR_copy(&ch->cur_peer_addr, peer_addr)) {
+        ch->addressed_mode = 0;
+        return 0;
+    }
+    ch->addressed_mode = 1;
+
     return 1;
 }
 
@@ -3671,7 +3674,9 @@ static int ch_on_new_conn_common(QUIC_CHANNEL *ch, const BIO_ADDR *peer,
                                  const QUIC_CONN_ID *peer_odcid)
 {
     /* Note our newly learnt peer address and CIDs. */
-    ch->cur_peer_addr   = *peer;
+    if (!BIO_ADDR_copy(&ch->cur_peer_addr, peer))
+        return 0;
+
     ch->init_dcid       = *peer_dcid;
     ch->cur_remote_dcid = *peer_scid;
     ch->odcid.id_len = 0;
diff --git a/ssl/quic/quic_record_tx.c b/ssl/quic/quic_record_tx.c
index cda684245d7..3f3688f134c 100644
--- a/ssl/quic/quic_record_tx.c
+++ b/ssl/quic/quic_record_tx.c
@@ -842,15 +842,19 @@ int ossl_qtx_write_pkt(OSSL_QTX *qtx, const OSSL_QTX_PKT *pkt)
 
         if (!was_coalescing) {
             /* Set addresses in TXE. */
-            if (pkt->peer != NULL)
-                txe->peer = *pkt->peer;
-            else
+            if (pkt->peer != NULL) {
+                if (!BIO_ADDR_copy(&txe->peer, pkt->peer))
+                    return 0;
+            } else {
                 BIO_ADDR_clear(&txe->peer);
+            }
 
-            if (pkt->local != NULL)
-                txe->local = *pkt->local;
-            else
+            if (pkt->local != NULL) {
+                if (!BIO_ADDR_copy(&txe->local, pkt->local))
+                    return 0;
+            } else {
                 BIO_ADDR_clear(&txe->local);
+            }
         }
 
         ret = qtx_mutate_write(qtx, pkt, txe, enc_level);
diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c
index 1d74637ffaa..55c41379bd9 100644
--- a/ssl/quic/quic_txp.c
+++ b/ssl/quic/quic_txp.c
@@ -705,8 +705,7 @@ int ossl_quic_tx_packetiser_set_peer(OSSL_QUIC_TX_PACKETISER *txp,
         return 1;
     }
 
-    txp->args.peer = *peer;
-    return 1;
+    return BIO_ADDR_copy(&txp->args.peer, peer);
 }
 
 void ossl_quic_tx_packetiser_set_ack_tx_cb(OSSL_QUIC_TX_PACKETISER *txp,