View Issue Details

IDProjectCategoryView StatusLast Update
0008820GNUnettransport servicepublic2024-06-08 10:03
Reporterulfvonbelow Assigned Toschanzen Due Date
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformx86-64OSGuix SystemOS Versiona1d711c92e
Product VersionGit master 
Fixed in Version0.21.2 
Summary0008820: Improper handling of natted address not being found in check_for_global_natted in gnunet-transport-service
DescriptionIf a matching address is found, contains_address will set tgna_cls.tgna and all will be fine. If one isn't, though, tgna_cls.tgna will be unaffected, and have whatever value it had before. tgna_cls is stack-allocated, so instead of this being NULL, it's undefined. The precise effect this will end up having therefore may not be the same on your machine, but on mine it caused the assertion
GNUNET_assert (neighbour->size_of_global_addresses >= ntohl (tgna_cls.tgna->address_length));
to fail.

Regardless of whether it's NULL or undefined, it shouldn't be dereferenced immediately afterward without some sort of check.

Checking for tgna_cls.tgna->address_length == 0 is also probably the wrong way of testing whether a tgna was found.
Steps To Reproduce1. Start transport service
2. ??? Maybe assertion failure?
Additional InformationPatch attached.

It is unclear to me whether tgna_cls.tgna->address_length is in host or network byte order. The GNUNET_log call uses tgna_cls.tgna->address_length without any translation, while elsewhere it is treated as if it is in network byte order.
TagsNo tags attached.
Attached Files
0001-TRANSPORT-properly-handle-no-matching-natted-address.patch (2,355 bytes)   
From c78b8f8567ae1ea288f66af557bccd2cf255ddab Mon Sep 17 00:00:00 2001
From: ulfvonbelow <striness@tilde.club>
Date: Sun, 5 May 2024 23:49:15 -0500
Subject: [PATCH] TRANSPORT: properly handle no matching natted address being
 found.

---
 .../transport/gnunet-service-transport.c       | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/service/transport/gnunet-service-transport.c b/src/service/transport/gnunet-service-transport.c
index 06c33ad00..35877e244 100644
--- a/src/service/transport/gnunet-service-transport.c
+++ b/src/service/transport/gnunet-service-transport.c
@@ -11738,18 +11738,20 @@ check_for_global_natted (void *cls,
   GNUNET_HELLO_builder_free (builder);
 
   tgna_cls.addr = get_address_without_port (queue->address);
+  tgna_cls.tgna = NULL;
   address_len_without_port = strlen (tgna_cls.addr);
   GNUNET_CONTAINER_multipeermap_get_multiple (neighbour->natted_addresses,
                                               &neighbour->pid,
                                               &contains_address,
                                               &tgna_cls);
-  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              " tgna_cls.tgna tgna %p %u %u %u\n",
-              tgna_cls.tgna,
-              neighbour->size_of_global_addresses,
-              tgna_cls.tgna->address_length,
-              neighbour->number_of_addresses);
-  if (0 == tgna_cls.tgna->address_length && GNUNET_YES == queue->is_global_natted)
+  if (NULL != tgna_cls.tgna)
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                " tgna_cls.tgna tgna %p %u %u %u\n",
+                tgna_cls.tgna,
+                neighbour->size_of_global_addresses,
+                tgna_cls.tgna->address_length,
+                neighbour->number_of_addresses);
+  if (NULL == tgna_cls.tgna && GNUNET_YES == queue->is_global_natted)
   {
     struct TransportGlobalNattedAddress *tgna;
 
@@ -11766,7 +11768,7 @@ check_for_global_natted (void *cls,
                 "Created tgna %p\n",
                 tgna);
   }
-  else if (0 != tgna_cls.tgna->address_length && GNUNET_NO == queue->is_global_natted)
+  else if (NULL != tgna_cls.tgna && GNUNET_NO == queue->is_global_natted)
   {
     GNUNET_CONTAINER_multipeermap_remove (neighbour->natted_addresses,
                                           &neighbour->pid,
-- 
2.41.0

Activities

schanzen

2024-06-08 10:03

administrator   ~0022529

0.21.2 released

Issue History

Date ModifiedUsernameFieldChange
2024-05-07 04:59ulfvonbelowNew Issue
2024-05-07 04:59ulfvonbelowFile Added: 0001-TRANSPORT-properly-handle-no-matching-natted-address.patch
2024-05-13 08:37schanzenAssigned To => t3sserakt
2024-05-13 08:37schanzenStatusnew => assigned
2024-05-14 08:30t3sseraktAssigned Tot3sserakt => schanzen
2024-05-14 08:30t3sseraktStatusassigned => resolved
2024-05-14 08:30t3sseraktResolutionopen => fixed
2024-05-14 15:21schanzenFixed in Version => 0.21.2
2024-06-08 10:03schanzenNote Added: 0022529
2024-06-08 10:03schanzenStatusresolved => closed