diff --git a/toxcore/BUILD.bazel b/toxcore/BUILD.bazel index 0f313e2af58..11fd791e5bb 100644 --- a/toxcore/BUILD.bazel +++ b/toxcore/BUILD.bazel @@ -478,10 +478,12 @@ cc_test( cc_fuzz_test( name = "DHT_fuzz_test", size = "small", + testonly = True, srcs = ["DHT_fuzz_test.cc"], corpus = ["//tools/toktok-fuzzer/corpus:DHT_fuzz_test"], deps = [ ":DHT", + ":DHT_test_util", "//c-toxcore/testing/fuzzing:fuzz_support", ], ) diff --git a/toxcore/DHT.c b/toxcore/DHT.c index 2567d1b5a8f..028525d0fc4 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -1938,7 +1938,9 @@ static int friend_iplist(const DHT *dht, IP_Port *ip_portlist, uint16_t friend_n } #ifdef FRIEND_IPLIST_PAD - memcpy(ip_portlist, ipv6s, num_ipv6s * sizeof(IP_Port)); + for (int i = 0; i < num_ipv6s; ++i) { + ip_portlist[i] = ipv6s[i]; + } if (num_ipv6s == MAX_FRIEND_CLIENTS) { return MAX_FRIEND_CLIENTS; @@ -1950,7 +1952,9 @@ static int friend_iplist(const DHT *dht, IP_Port *ip_portlist, uint16_t friend_n num_ipv4s_used = num_ipv4s; } - memcpy(&ip_portlist[num_ipv6s], ipv4s, num_ipv4s_used * sizeof(IP_Port)); + for (int i = 0; i < num_ipv4s_used; ++i) { + ip_portlist[num_ipv6s + i] = ipv4s[i]; + } return num_ipv6s + num_ipv4s_used; #else /* !FRIEND_IPLIST_PAD */ @@ -1959,11 +1963,15 @@ static int friend_iplist(const DHT *dht, IP_Port *ip_portlist, uint16_t friend_n * with the shorter one... */ if (num_ipv6s >= num_ipv4s) { - memcpy(ip_portlist, ipv6s, num_ipv6s * sizeof(IP_Port)); + for (int i = 0; i < num_ipv6s; ++i) { + ip_portlist[i] = ipv6s[i]; + } return num_ipv6s; } - memcpy(ip_portlist, ipv4s, num_ipv4s * sizeof(IP_Port)); + for (int i = 0; i < num_ipv4s; ++i) { + ip_portlist[i] = ipv4s[i]; + } return num_ipv4s; #endif /* !FRIEND_IPLIST_PAD */ @@ -2539,7 +2547,7 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw dht->hole_punching_enabled = hole_punching_enabled; dht->lan_discovery_enabled = lan_discovery_enabled; - dht->ping = ping_new(mem, mono_time, rng, dht); + dht->ping = ping_new(mem, mono_time, log, rng, dht); if (dht->ping == nullptr) { LOGGER_ERROR(log, "failed to initialise ping"); diff --git a/toxcore/DHT_fuzz_test.cc b/toxcore/DHT_fuzz_test.cc index 868aedecf2a..b0ce362417a 100644 --- a/toxcore/DHT_fuzz_test.cc +++ b/toxcore/DHT_fuzz_test.cc @@ -6,6 +6,7 @@ #include #include "../testing/fuzzing/fuzz_support.hh" +#include "DHT_test_util.hh" namespace { @@ -26,31 +27,32 @@ void TestUnpackNodes(Fuzz_Data &input) CONSUME1_OR_RETURN(const bool, tcp_enabled, input); const uint16_t node_count = 5; - Node_format nodes[node_count]; + std::array nodes; uint16_t processed_data_len; const int packed_count = unpack_nodes( - nodes, node_count, &processed_data_len, input.data(), input.size(), tcp_enabled); + nodes.data(), nodes.size(), &processed_data_len, input.data(), input.size(), tcp_enabled); if (packed_count > 0) { Logger *logger = logger_new(); std::vector packed(packed_count * PACKED_NODE_SIZE_IP6); const int packed_size - = pack_nodes(logger, packed.data(), packed.size(), nodes, packed_count); + = pack_nodes(logger, packed.data(), packed.size(), nodes.data(), packed_count); LOGGER_ASSERT(logger, packed_size == processed_data_len, "packed size (%d) != unpacked size (%d)", packed_size, processed_data_len); logger_kill(logger); // Check that packed nodes can be unpacked again and result in the // original unpacked nodes. - Node_format nodes2[node_count]; + std::array nodes2; uint16_t processed_data_len2; - const int packed_count2 = unpack_nodes( - nodes2, node_count, &processed_data_len2, packed.data(), packed.size(), tcp_enabled); + const int packed_count2 = unpack_nodes(nodes2.data(), nodes2.size(), &processed_data_len2, + packed.data(), packed.size(), tcp_enabled); (void)packed_count2; #if 0 assert(processed_data_len2 == processed_data_len); assert(packed_count2 == packed_count); #endif - assert(memcmp(nodes, nodes2, sizeof(Node_format) * packed_count) == 0); + assert(std::vector(nodes.begin(), nodes.begin() + packed_count) + == std::vector(nodes2.begin(), nodes2.begin() + packed_count)); } } diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index 947edef7e12..21e82ead30e 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -2591,7 +2591,7 @@ static bool self_announce_group(const Messenger *m, GC_Chat *chat, Onion_Friend announce.base_announce.ip_port_is_set = ip_port_is_set; if (ip_port_is_set) { - memcpy(&announce.base_announce.ip_port, &chat->self_ip_port, sizeof(IP_Port)); + announce.base_announce.ip_port = chat->self_ip_port; } memcpy(announce.base_announce.peer_public_key, chat->self_public_key.enc, ENC_PUBLIC_KEY_SIZE); diff --git a/toxcore/network.c b/toxcore/network.c index 55aa4e28184..f7e560ca132 100644 --- a/toxcore/network.c +++ b/toxcore/network.c @@ -933,7 +933,7 @@ int send_packet(const Networking_Core *net, const IP_Port *ip_port, Packet packe { IP_Port ipp_copy = *ip_port; - if (net_family_is_unspec(ip_port->ip.family)) { + if (net_family_is_unspec(ipp_copy.ip.family)) { // TODO(iphydf): Make this an error. Currently this fails sometimes when // called from DHT.c:do_ping_and_sendnode_requests. return -1; @@ -996,7 +996,7 @@ int send_packet(const Networking_Core *net, const IP_Port *ip_port, Packet packe } const long res = net_sendto(net->ns, net->sock, packet.data, packet.length, &addr, &ipp_copy); - loglogdata(net->log, "O=>", packet.data, packet.length, ip_port, res); + loglogdata(net->log, "O=>", packet.data, packet.length, &ipp_copy, res); assert(res <= INT_MAX); return (int)res; @@ -1021,7 +1021,8 @@ int sendpacket(const Networking_Core *net, const IP_Port *ip_port, const uint8_t non_null() static int receivepacket(const Network *ns, const Memory *mem, const Logger *log, Socket sock, IP_Port *ip_port, uint8_t *data, uint32_t *length) { - memset(ip_port, 0, sizeof(IP_Port)); + ipport_reset(ip_port); + Network_Addr addr = {{0}}; addr.size = sizeof(addr.addr); *length = 0; @@ -1514,13 +1515,25 @@ void ip_reset(IP *ip) static const IP_Port empty_ip_port = {{{0}}}; /** nulls out ip_port */ -void ipport_reset(IP_Port *ipport) +void ipport_reset(IP_Port *ip_port) { - if (ipport == nullptr) { + if (ip_port == nullptr) { return; } - *ipport = empty_ip_port; +#ifdef RANDOM_PADDING + // Leave padding bytes as uninitialized data. This should not matter, because we + // then set all the actual fields to 0. + IP_Port empty; + empty.ip.family.value = 0; + empty.ip.ip.v6.uint64[0] = 0; + empty.ip.ip.v6.uint64[1] = 0; + empty.port = 0; + + *ip_port = empty; +#else + *ip_port = empty_ip_port; +#endif /* RANDOM_PADDING */ } /** nulls out ip, sets family according to flag */ @@ -1630,7 +1643,7 @@ bool bin_pack_ip_port(Bin_Pack *bp, const Logger *logger, const IP_Port *ip_port Ip_Ntoa ip_str; // TODO(iphydf): Find out why we're trying to pack invalid IPs, stop // doing that, and turn this into an error. - LOGGER_TRACE(logger, "cannot pack invalid IP: %s", net_ip_ntoa(&ip_port->ip, &ip_str)); + LOGGER_DEBUG(logger, "cannot pack invalid IP: %s", net_ip_ntoa(&ip_port->ip, &ip_str)); return false; } @@ -1651,6 +1664,7 @@ int pack_ip_port(const Logger *logger, uint8_t *data, uint16_t length, const IP_ const uint32_t size = bin_pack_obj_size(bin_pack_ip_port_handler, ip_port, logger); if (size > length) { + LOGGER_ERROR(logger, "not enough space for packed IP: %u but need %u", length, size); return -1; } diff --git a/toxcore/network.h b/toxcore/network.h index 06857b89179..bd540abdb30 100644 --- a/toxcore/network.h +++ b/toxcore/network.h @@ -378,7 +378,7 @@ non_null() void ip_reset(IP *ip); /** nulls out ip_port */ non_null() -void ipport_reset(IP_Port *ipport); +void ipport_reset(IP_Port *ip_port); /** nulls out ip, sets family according to flag */ non_null() void ip_init(IP *ip, bool ipv6enabled); diff --git a/toxcore/onion_announce.c b/toxcore/onion_announce.c index 593d81aa2ca..2d40ec1964f 100644 --- a/toxcore/onion_announce.c +++ b/toxcore/onion_announce.c @@ -24,6 +24,7 @@ #include "onion.h" #include "shared_key_cache.h" #include "timed_auth.h" +#include "util.h" #define PING_ID_TIMEOUT ONION_ANNOUNCE_TIMEOUT @@ -463,10 +464,11 @@ static int handle_announce_request_common( return 1; } - const uint16_t ping_id_data_len = CRYPTO_PUBLIC_KEY_SIZE + sizeof(*source); - uint8_t ping_id_data[CRYPTO_PUBLIC_KEY_SIZE + sizeof(*source)]; + const uint16_t ping_id_data_len = CRYPTO_PUBLIC_KEY_SIZE + SIZE_IPPORT; + uint8_t ping_id_data[CRYPTO_PUBLIC_KEY_SIZE + SIZE_IPPORT]; memcpy(ping_id_data, packet_public_key, CRYPTO_PUBLIC_KEY_SIZE); - memcpy(ping_id_data + CRYPTO_PUBLIC_KEY_SIZE, source, sizeof(*source)); + const int packed_len = pack_ip_port(onion_a->log, &ping_id_data[CRYPTO_PUBLIC_KEY_SIZE], SIZE_IPPORT, source); + memzero(&ping_id_data[CRYPTO_PUBLIC_KEY_SIZE + packed_len], SIZE_IPPORT - packed_len); const uint8_t *data_public_key = plain + ONION_PING_ID_SIZE + CRYPTO_PUBLIC_KEY_SIZE; @@ -510,7 +512,7 @@ static int handle_announce_request_common( int nodes_length = 0; if (num_nodes != 0) { - nodes_length = pack_nodes(onion_a->log, response + nodes_offset, sizeof(nodes_list), nodes_list, + nodes_length = pack_nodes(onion_a->log, &response[nodes_offset], num_nodes * PACKED_NODE_SIZE_IP6, nodes_list, (uint16_t)num_nodes); if (nodes_length <= 0) { diff --git a/toxcore/onion_client.c b/toxcore/onion_client.c index 9b0ac96102a..91f026a9d45 100644 --- a/toxcore/onion_client.c +++ b/toxcore/onion_client.c @@ -576,11 +576,12 @@ non_null() static int new_sendback(Onion_Client *onion_c, uint32_t num, const uint8_t *public_key, const IP_Port *ip_port, uint32_t path_num, uint64_t *sendback) { - uint8_t data[sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE + sizeof(IP_Port) + sizeof(uint32_t)]; + uint8_t data[sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE + SIZE_IPPORT + sizeof(uint32_t)]; memcpy(data, &num, sizeof(uint32_t)); - memcpy(data + sizeof(uint32_t), public_key, CRYPTO_PUBLIC_KEY_SIZE); - memcpy(data + sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE, ip_port, sizeof(IP_Port)); - memcpy(data + sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE + sizeof(IP_Port), &path_num, sizeof(uint32_t)); + memcpy(&data[sizeof(uint32_t)], public_key, CRYPTO_PUBLIC_KEY_SIZE); + const int packed_len = pack_ip_port(onion_c->logger, &data[sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE], SIZE_IPPORT, ip_port); + memzero(&data[sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE + packed_len], SIZE_IPPORT - packed_len); + memcpy(&data[sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE + SIZE_IPPORT], &path_num, sizeof(uint32_t)); *sendback = ping_array_add(onion_c->announce_ping_array, onion_c->mono_time, onion_c->rng, data, sizeof(data)); if (*sendback == 0) { @@ -607,15 +608,15 @@ static uint32_t check_sendback(Onion_Client *onion_c, const uint8_t *sendback, u { uint64_t sback; memcpy(&sback, sendback, sizeof(uint64_t)); - uint8_t data[sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE + sizeof(IP_Port) + sizeof(uint32_t)]; + uint8_t data[sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE + SIZE_IPPORT + sizeof(uint32_t)]; if (ping_array_check(onion_c->announce_ping_array, onion_c->mono_time, data, sizeof(data), sback) != sizeof(data)) { return -1; } memcpy(ret_pubkey, data + sizeof(uint32_t), CRYPTO_PUBLIC_KEY_SIZE); - memcpy(ret_ip_port, data + sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE, sizeof(IP_Port)); - memcpy(path_num, data + sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE + sizeof(IP_Port), sizeof(uint32_t)); + unpack_ip_port(ret_ip_port, data + sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE, SIZE_IPPORT, false); + memcpy(path_num, data + sizeof(uint32_t) + CRYPTO_PUBLIC_KEY_SIZE + SIZE_IPPORT, sizeof(uint32_t)); uint32_t num; memcpy(&num, data, sizeof(uint32_t)); diff --git a/toxcore/ping.c b/toxcore/ping.c index 303c418ceac..fdd131cd23a 100644 --- a/toxcore/ping.c +++ b/toxcore/ping.c @@ -15,10 +15,12 @@ #include "attributes.h" #include "ccompat.h" #include "crypto_core.h" +#include "logger.h" #include "mem.h" #include "mono_time.h" #include "network.h" #include "ping_array.h" +#include "util.h" #define PING_NUM_MAX 512 @@ -30,6 +32,7 @@ struct Ping { const Mono_Time *mono_time; + const Logger *log; const Random *rng; DHT *dht; @@ -40,7 +43,7 @@ struct Ping { #define PING_PLAIN_SIZE (1 + sizeof(uint64_t)) #define DHT_PING_SIZE (1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE + PING_PLAIN_SIZE + CRYPTO_MAC_SIZE) -#define PING_DATA_SIZE (CRYPTO_PUBLIC_KEY_SIZE + sizeof(IP_Port)) +#define PING_DATA_SIZE (CRYPTO_PUBLIC_KEY_SIZE + SIZE_IPPORT) void ping_send_request(Ping *ping, const IP_Port *ipp, const uint8_t *public_key) { @@ -57,7 +60,8 @@ void ping_send_request(Ping *ping, const IP_Port *ipp, const uint8_t *public_key // Generate random ping_id. uint8_t data[PING_DATA_SIZE]; pk_copy(data, public_key); - memcpy(data + CRYPTO_PUBLIC_KEY_SIZE, ipp, sizeof(IP_Port)); + const int packed_len = pack_ip_port(ping->log, &data[CRYPTO_PUBLIC_KEY_SIZE], PING_DATA_SIZE - CRYPTO_PUBLIC_KEY_SIZE, ipp); + memzero(&data[packed_len], SIZE_IPPORT - packed_len); ping_id = ping_array_add(ping->ping_array, ping->mono_time, ping->rng, data, sizeof(data)); if (ping_id == 0) { @@ -209,7 +213,7 @@ static int handle_ping_response(void *object, const IP_Port *source, const uint8 } IP_Port ipp; - memcpy(&ipp, data + CRYPTO_PUBLIC_KEY_SIZE, sizeof(IP_Port)); + unpack_ip_port(&ipp, &data[CRYPTO_PUBLIC_KEY_SIZE], PING_DATA_SIZE - CRYPTO_PUBLIC_KEY_SIZE, false); if (!ipport_equal(&ipp, source)) { return 1; @@ -331,7 +335,7 @@ void ping_iterate(Ping *ping) } } -Ping *ping_new(const Memory *mem, const Mono_Time *mono_time, const Random *rng, DHT *dht) +Ping *ping_new(const Memory *mem, const Mono_Time *mono_time, const Logger *log, const Random *rng, DHT *dht) { Ping *ping = (Ping *)mem_alloc(mem, sizeof(Ping)); @@ -347,6 +351,7 @@ Ping *ping_new(const Memory *mem, const Mono_Time *mono_time, const Random *rng, } ping->mono_time = mono_time; + ping->log = log; ping->rng = rng; ping->dht = dht; networking_registerhandler(dht_get_net(ping->dht), NET_PACKET_PING_REQUEST, &handle_ping_request, dht); diff --git a/toxcore/ping.h b/toxcore/ping.h index b0339ec75d0..45eaeca6925 100644 --- a/toxcore/ping.h +++ b/toxcore/ping.h @@ -22,7 +22,7 @@ typedef struct Ping Ping; non_null() -Ping *ping_new(const Memory *mem, const Mono_Time *mono_time, const Random *rng, DHT *dht); +Ping *ping_new(const Memory *mem, const Mono_Time *mono_time, const Logger *log, const Random *rng, DHT *dht); non_null(1) nullable(2) void ping_kill(const Memory *mem, Ping *ping);