gnunet

commit 5e9be519a5336da5056f0f2c67f43c9e9ebf1bea
parent 6110113a4ac568485f3a9fbdf7c11616173dab7a
Author: Martin Schanzenbach <schanzen@gnunet.org>
Date:   Sat,  1 Aug 2026 07:35:04 +0200

transport: validate other peer hellos

Diffstat:
M
src/service/transport/gnunet-service-transport.c
 | 
94
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------

1 file changed, 77 insertions(+), 17 deletions(-)
diff --git a/src/service/transport/gnunet-service-transport.c b/src/service/transport/gnunet-service-transport.c
@@ -10315,6 +10315,50 @@ hello_for_incoming_cb (void *cls,
 
 
 /**
+ * Obtain the HELLO message stored in @a record, if @a record actually
+ * carries one.
+ *
+ * PEERSTORE values are opaque blobs: #handle_store() of the PEERSTORE
+ * service performs no validation whatsoever, and the records under
+ * #GNUNET_PEERSTORE_HELLO_KEY originate from other peers (via the DHT,
+ * the hostlist daemon and topology gossip).  So the length of the value
+ * and the length the embedded message *claims* to have are two
+ * independent numbers, and neither may be trusted:
+ *
+ * - a zero-length value leaves @e value at NULL (see
+ *   #PEERSTORE_parse_record_message()), and
+ *   #GNUNET_HELLO_parser_from_msg() dereferences its argument
+ *   unconditionally -- that is a NULL dereference;
+ * - a value shorter than a message header, or one whose @e size field
+ *   exceeds @e value_size, makes the parser read past the end of the
+ *   heap block the record was parsed into.
+ *
+ * @param record record to inspect, may be NULL
+ * @return the HELLO, or NULL if @a record does not hold a well-formed one
+ */
+static const struct GNUNET_MessageHeader *
+hello_from_record (const struct GNUNET_PEERSTORE_Record *record)
+{
+  const struct GNUNET_MessageHeader *hello;
+
+  if ((NULL == record) ||
+      (NULL == record->value) ||
+      (record->value_size < sizeof(*hello)))
+  {
+    GNUNET_break_op (0);
+    return NULL;
+  }
+  hello = record->value;
+  if (ntohs (hello->size) > record->value_size)
+  {
+    GNUNET_break_op (0);
+    return NULL;
+  }
+  return hello;
+}
+
+
+/**
  * Function called by PEERSTORE for each matching record.
  *
  * @param cls closure, a `struct IncomingRequest`
@@ -10328,7 +10372,7 @@ handle_hello_for_incoming (void *cls,
 {
   struct IncomingRequest *ir = cls;
   struct GNUNET_HELLO_Parser *parser;
-  struct GNUNET_MessageHeader *hello;
+  const struct GNUNET_MessageHeader *hello;
   const struct GNUNET_PeerIdentity *my_identity;
 
   if (NULL != emsg)
@@ -10343,7 +10387,14 @@ handle_hello_for_incoming (void *cls,
     GNUNET_break (0);
     return;
   }
-  hello = record->value;
+  hello = hello_from_record (record);
+  if (NULL == hello)
+  {
+    /* MUST still ask for the next record: a bad one is not a reason to
+       stall this monitor forever. */
+    GNUNET_PEERSTORE_monitor_next (ir->nc, 1);
+    return;
+  }
   my_identity = GNUNET_PILS_get_identity (pils);
   GNUNET_assert (my_identity);
   if (0 == GNUNET_memcmp (&record->peer, my_identity))
@@ -11135,7 +11186,7 @@ check_for_burst_address (void *cls,
 {
   struct GNUNET_StartBurstCls *sb_cls = cls;
   struct VirtualLink *vl = sb_cls->vl;
-  struct GNUNET_MessageHeader *hello;
+  const struct GNUNET_MessageHeader *hello;
   struct GNUNET_HELLO_Parser *parser;
 
   if (NULL != emsg)
@@ -11166,15 +11217,18 @@ check_for_burst_address (void *cls,
 
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
               "check_for_burst_address\n");
-  hello = record->value;
   /* @a hello was published by a remote peer, it may well be malformed. */
-  parser = GNUNET_HELLO_parser_from_msg (hello, &record->peer);
-  if (NULL != parser)
+  hello = hello_from_record (record);
+  if (NULL != hello)
   {
-    GNUNET_HELLO_parser_iterate (parser,
-                                 &iterate_address_start_burst,
-                                 vl);
-    GNUNET_HELLO_parser_free (parser);
+    parser = GNUNET_HELLO_parser_from_msg (hello, &record->peer);
+    if (NULL != parser)
+    {
+      GNUNET_HELLO_parser_iterate (parser,
+                                   &iterate_address_start_burst,
+                                   vl);
+      GNUNET_HELLO_parser_free (parser);
+    }
   }
 
   /* MUST clear @e ic: #GNUNET_PEERSTORE_iteration_stop() frees it, and
@@ -13950,7 +14004,7 @@ check_for_global_natted (void *cls,
   struct Queue *queue = cls;
   struct Neighbour *neighbour = queue->neighbour;
   struct GNUNET_HELLO_Parser *parser;
-  struct GNUNET_MessageHeader *hello;
+  const struct GNUNET_MessageHeader *hello;
   struct TransportGlobalNattedAddressClosure tgna_cls;
   size_t address_len_without_port;
 
@@ -13966,15 +14020,14 @@ check_for_global_natted (void *cls,
     GNUNET_break (0);
     return;
   }
-  if (0 == record->value_size)
+  /* @a hello was published by a remote peer and may be malformed. */
+  hello = hello_from_record (record);
+  if (NULL == hello)
   {
     GNUNET_PEERSTORE_monitor_next (queue->mo, 1);
-    GNUNET_break (0);
     return;
   }
   queue->is_global_natted = GNUNET_YES;
-  hello = record->value;
-  /* @a hello was published by a remote peer and may be malformed. */
   parser = GNUNET_HELLO_parser_from_msg (hello, &record->peer);
   if (NULL == parser)
   {
@@ -14450,7 +14503,7 @@ handle_hello_for_client (void *cls,
   const struct GNUNET_PeerIdentity *my_identity;
   struct PeerRequest *pr = cls;
   struct GNUNET_HELLO_Parser *parser;
-  struct GNUNET_MessageHeader *hello;
+  const struct GNUNET_MessageHeader *hello;
 
   if (NULL != emsg)
   {
@@ -14471,7 +14524,14 @@ handle_hello_for_client (void *cls,
                 "No identity given yet!\n");
     return;
   }
-  hello = record->value;
+  hello = hello_from_record (record);
+  if (NULL == hello)
+  {
+    /* MUST still ask for the next record: a bad one is not a reason to
+       stall this monitor forever. */
+    GNUNET_PEERSTORE_monitor_next (pr->nc, 1);
+    return;
+  }
   if (0 == GNUNET_memcmp (&record->peer, my_identity))
   {
     GNUNET_PEERSTORE_monitor_next (pr->nc, 1);