View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0008818 | GNUnet | transport service | public | 2024-05-07 06:26 | 2024-06-08 12:03 |
| Reporter | ulfvonbelow | Assigned To | schanzen | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Platform | x86-64 | OS | Guix System | OS Version | a1d711c92e |
| Product Version | Git master | ||||
| Fixed in Version | 0.21.2 | ||||
| Summary | 0008818: consider_sending_fc in gnunet-service-transport incorrectly populates the addresses of its TransportFlowControlMessage | ||||
| Description | In add_global_addresses, off += ... at the very end has no effect whatsoever, because off goes out of scope as soon as add_global_addresses returns and is recreated in a different stack frame the next time it is called, set to 0 once more. Consequently, the addresses will all be copied to the start of the output buffer. If there are multiple addresses, this means that the output will be a combination of the final address to be iterated over and some suffixes from a subset of the addresses coming before it, plus a lot of zero bytes. | ||||
| Steps To Reproduce | Thought experiment, I only came across this while debugging something else. | ||||
| Additional Information | Patch attached. | ||||
| Tags | No tags attached. | ||||
| Attached Files | 0001-TRANSPORT-fix-add_global_addresses.patch (3,374 bytes)
From 26ddeacf0fe663630bb3f9e86c6fb85e33279544 Mon Sep 17 00:00:00 2001
From: ulfvonbelow <striness@tilde.club>
Date: Sun, 5 May 2024 23:43:04 -0500
Subject: [PATCH] TRANSPORT: fix add_global_addresses.
Previously the bytes of the current iteration's address would simply overwrite
the start of the output buffer each time. This meant that when a peer had
multiple global addresses, some would be missing and/or mangled.
---
.../transport/gnunet-service-transport.c | 24 ++++++++++++-------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/service/transport/gnunet-service-transport.c b/src/service/transport/gnunet-service-transport.c
index 311f269b7..38371032e 100644
--- a/src/service/transport/gnunet-service-transport.c
+++ b/src/service/transport/gnunet-service-transport.c
@@ -5345,16 +5345,21 @@ static char *
get_address_without_port (const char *address);
+struct AddGlobalAddressesContext
+{
+ size_t off;
+ char *tgnas;
+};
+
static enum GNUNET_GenericReturnValue
add_global_addresses (void *cls,
const struct GNUNET_PeerIdentity *pid,
void *value)
{
- char *tgnas = cls;
+ struct AddGlobalAddressesContext *ctx = cls;
struct TransportGlobalNattedAddress *tgna = value;
char *addr = (char *) &tgna[1];
size_t address_len = strlen (addr);
- unsigned int off = 0;
GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
"sending address %s length %u\n",
@@ -5363,9 +5368,9 @@ add_global_addresses (void *cls,
tgna = GNUNET_malloc (sizeof (struct TransportGlobalNattedAddress) + address_len);
tgna->address_length = htonl (address_len);
GNUNET_memcpy (&tgna[1], addr, address_len);
- GNUNET_memcpy (&tgnas[off], tgna, sizeof (struct TransportGlobalNattedAddress) + address_len);
+ GNUNET_memcpy (&(ctx->tgnas[ctx->off]), tgna, sizeof (struct TransportGlobalNattedAddress) + address_len);
GNUNET_free (tgna);
- off += sizeof(struct TransportGlobalNattedAddress) + address_len;
+ ctx->off += sizeof(struct TransportGlobalNattedAddress) + address_len;
}
@@ -5387,17 +5392,20 @@ consider_sending_fc (void *cls)
if (0 < n->number_of_addresses)
{
- char *tgnas = GNUNET_malloc (n->number_of_addresses * sizeof (struct TransportGlobalNattedAddress) + n->size_of_global_addresses);
- size_t addresses_size;
+ size_t addresses_size =
+ n->number_of_addresses * sizeof (struct TransportGlobalNattedAddress) + n->size_of_global_addresses;
+ char *tgnas = GNUNET_malloc (addresses_size);
+ struct AddGlobalAddressesContext ctx;
+ ctx.off = 0;
+ ctx.tgnas = tgnas;
- addresses_size = n->number_of_addresses * sizeof (struct TransportGlobalNattedAddress) + n->size_of_global_addresses;
fc = GNUNET_malloc (sizeof (struct TransportFlowControlMessage) + addresses_size);
fc->header.size = htons (sizeof(struct TransportFlowControlMessage) + addresses_size);
fc->size_of_addresses = htonl (n->size_of_global_addresses);
fc->number_of_addresses = htonl (n->number_of_addresses);
GNUNET_CONTAINER_multipeermap_iterate (n->natted_addresses,
&add_global_addresses,
- tgnas);
+ &ctx);
GNUNET_memcpy (&fc[1], tgnas, addresses_size);
GNUNET_free (tgnas);
}
--
2.41.0
| ||||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2024-05-07 06:26 | ulfvonbelow | New Issue | |
| 2024-05-07 06:26 | ulfvonbelow | File Added: 0001-TRANSPORT-fix-add_global_addresses.patch | |
| 2024-05-13 10:38 | schanzen | Assigned To | => t3sserakt |
| 2024-05-13 10:38 | schanzen | Status | new => assigned |
| 2024-05-14 10:39 | t3sserakt | Assigned To | t3sserakt => schanzen |
| 2024-05-14 10:39 | t3sserakt | Status | assigned => resolved |
| 2024-05-14 10:39 | t3sserakt | Resolution | open => fixed |
| 2024-05-14 17:20 | schanzen | Fixed in Version | => 0.21.2 |
| 2024-06-08 12:03 | schanzen | Note Added: 0022531 | |
| 2024-06-08 12:03 | schanzen | Status | resolved => closed |