diff --git a/auto_tests/network_test.c b/auto_tests/network_test.c index 6d105262a..0e5d8d3b9 100644 --- a/auto_tests/network_test.c +++ b/auto_tests/network_test.c @@ -35,11 +35,11 @@ static void test_addr_resolv_localhost(void) ck_assert_msg(res, "Resolver failed: %d, %s", error, strerror); net_kill_strerror(strerror); - char ip_str[IP_NTOA_LEN]; + Ip_Ntoa ip_str; ck_assert_msg(net_family_is_ipv4(ip.family), "Expected family TOX_AF_INET, got %u.", ip.family.value); const uint32_t loopback = get_ip4_loopback().uint32; ck_assert_msg(ip.ip.v4.uint32 == loopback, "Expected 127.0.0.1, got %s.", - ip_ntoa(&ip, ip_str, sizeof(ip_str))); + net_ip_ntoa(&ip, &ip_str)); ip_init(&ip, 1); // ipv6enabled = 1 res = addr_resolve_or_parse_ip(ns, localhost, &ip, nullptr); @@ -62,7 +62,7 @@ static void test_addr_resolv_localhost(void) ip.family.value); IP6 ip6_loopback = get_ip6_loopback(); ck_assert_msg(!memcmp(&ip.ip.v6, &ip6_loopback, sizeof(IP6)), "Expected ::1, got %s.", - ip_ntoa(&ip, ip_str, sizeof(ip_str))); + net_ip_ntoa(&ip, &ip_str)); if (localhost_split) { printf("Localhost seems to be split in two.\n"); @@ -85,17 +85,17 @@ static void test_addr_resolv_localhost(void) ck_assert_msg(net_family_is_ipv6(ip.family), "Expected family TOX_AF_INET6 (%d), got %u.", TOX_AF_INET6, ip.family.value); ck_assert_msg(!memcmp(&ip.ip.v6, &ip6_loopback, sizeof(IP6)), "Expected ::1, got %s.", - ip_ntoa(&ip, ip_str, sizeof(ip_str))); + net_ip_ntoa(&ip, &ip_str)); ck_assert_msg(net_family_is_ipv4(extra.family), "Expected family TOX_AF_INET (%d), got %u.", TOX_AF_INET, extra.family.value); ck_assert_msg(extra.ip.v4.uint32 == loopback, "Expected 127.0.0.1, got %s.", - ip_ntoa(&ip, ip_str, sizeof(ip_str))); + net_ip_ntoa(&ip, &ip_str)); #elif 0 // TODO(iphydf): Fix this to work on IPv6-supporting systems. ck_assert_msg(net_family_is_ipv4(ip.family), "Expected family TOX_AF_INET (%d), got %u.", TOX_AF_INET, ip.family.value); ck_assert_msg(ip.ip.v4.uint32 == loopback, "Expected 127.0.0.1, got %s.", - ip_ntoa(&ip, ip_str, sizeof(ip_str))); + net_ip_ntoa(&ip, &ip_str)); #endif } diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index b38b7f574..b8d600c5f 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -4a2bfe7abf6060f252154062818541e485344b350cf975f7aeea78ff249bfa65 /usr/local/bin/tox-bootstrapd +f3781691aed256efccda556f5663a4ab95c460aa8c1d665667c8b80c44e4d630 /usr/local/bin/tox-bootstrapd diff --git a/toxcore/DHT.c b/toxcore/DHT.c index 145b2e971..513fcb887 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -482,10 +482,10 @@ int pack_ip_port(const Logger *logger, uint8_t *data, uint16_t length, const IP_ is_ipv4 = false; family = TOX_TCP_INET6; } else { - char ip_str[IP_NTOA_LEN]; + 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", ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str))); + LOGGER_TRACE(logger, "cannot pack invalid IP: %s", net_ip_ntoa(&ip_port->ip, &ip_str)); return -1; } @@ -781,12 +781,13 @@ static void update_client(const Logger *log, const Mono_Time *mono_time, int ind } if (!ipport_equal(&assoc->ip_port, ip_port)) { - char ip_str[IP_NTOA_LEN]; + Ip_Ntoa ip_str_from; + Ip_Ntoa ip_str_to; LOGGER_TRACE(log, "coipil[%u]: switching ipv%d from %s:%u to %s:%u", index, ip_version, - ip_ntoa(&assoc->ip_port.ip, ip_str, sizeof(ip_str)), + net_ip_ntoa(&assoc->ip_port.ip, &ip_str_from), net_ntohs(assoc->ip_port.port), - ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str)), + net_ip_ntoa(&ip_port->ip, &ip_str_to), net_ntohs(ip_port->port)); } diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index d18f0a810..016571cb7 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -2419,10 +2419,10 @@ void do_messenger(Messenger *m, void *userdata) last_pinged = 999; } - char ip_str[IP_NTOA_LEN]; + Ip_Ntoa ip_str; char id_str[IDSTRING_LEN]; LOGGER_TRACE(m->log, "C[%2u] %s:%u [%3u] %s", - client, ip_ntoa(&assoc->ip_port.ip, ip_str, sizeof(ip_str)), + client, net_ip_ntoa(&assoc->ip_port.ip, &ip_str), net_ntohs(assoc->ip_port.port), last_pinged, id_to_string(cptr->public_key, id_str, sizeof(id_str))); } @@ -2492,10 +2492,10 @@ void do_messenger(Messenger *m, void *userdata) last_pinged = 999; } - char ip_str[IP_NTOA_LEN]; + Ip_Ntoa ip_str; char id_str[IDSTRING_LEN]; LOGGER_TRACE(m->log, "F[%2u] => C[%2u] %s:%u [%3u] %s", - friend_idx, client, ip_ntoa(&assoc->ip_port.ip, ip_str, sizeof(ip_str)), + friend_idx, client, net_ip_ntoa(&assoc->ip_port.ip, &ip_str), net_ntohs(assoc->ip_port.port), last_pinged, id_to_string(cptr->public_key, id_str, sizeof(id_str))); } diff --git a/toxcore/network.c b/toxcore/network.c index 6466a8a1d..0f9e48ee8 100644 --- a/toxcore/network.c +++ b/toxcore/network.c @@ -670,25 +670,25 @@ static void loglogdata(const Logger *log, const char *message, const uint8_t *bu uint16_t buflen, const IP_Port *ip_port, long res) { if (res < 0) { /* Windows doesn't necessarily know `%zu` */ - char ip_str[IP_NTOA_LEN]; + Ip_Ntoa ip_str; const int error = net_error(); char *strerror = net_new_strerror(error); LOGGER_TRACE(log, "[%2u] %s %3u%c %s:%u (%u: %s) | %08x%08x...%02x", buffer[0], message, min_u16(buflen, 999), 'E', - ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str)), net_ntohs(ip_port->port), error, + net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port), error, strerror, data_0(buflen, buffer), data_1(buflen, buffer), buffer[buflen - 1]); net_kill_strerror(strerror); } else if ((res > 0) && ((size_t)res <= buflen)) { - char ip_str[IP_NTOA_LEN]; + Ip_Ntoa ip_str; LOGGER_TRACE(log, "[%2u] %s %3u%c %s:%u (%u: %s) | %08x%08x...%02x", buffer[0], message, min_u16(res, 999), (size_t)res < buflen ? '<' : '=', - ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str)), net_ntohs(ip_port->port), 0, "OK", + net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port), 0, "OK", data_0(buflen, buffer), data_1(buflen, buffer), buffer[buflen - 1]); } else { /* empty or overwrite */ - char ip_str[IP_NTOA_LEN]; + Ip_Ntoa ip_str; LOGGER_TRACE(log, "[%2u] %s %lu%c%u %s:%u (%u: %s) | %08x%08x...%02x", buffer[0], message, res, res == 0 ? '!' : '>', buflen, - ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str)), net_ntohs(ip_port->port), 0, "OK", + net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port), 0, "OK", data_0(buflen, buffer), data_1(buflen, buffer), buffer[buflen - 1]); } } @@ -1198,8 +1198,8 @@ Networking_Core *new_networking_ex( if (res == 0) { temp->port = *portptr; - char ip_str[IP_NTOA_LEN]; - LOGGER_DEBUG(log, "Bound successfully to %s:%u", ip_ntoa(ip, ip_str, sizeof(ip_str)), + Ip_Ntoa ip_str; + LOGGER_DEBUG(log, "Bound successfully to %s:%u", net_ip_ntoa(ip, &ip_str), net_ntohs(temp->port)); /* errno isn't reset on success, only set on failure, the failed @@ -1225,11 +1225,11 @@ Networking_Core *new_networking_ex( *portptr = net_htons(port_to_try); } - char ip_str[IP_NTOA_LEN]; + Ip_Ntoa ip_str; int neterror = net_error(); char *strerror = net_new_strerror(neterror); LOGGER_ERROR(log, "failed to bind socket: %d, %s IP: %s port_from: %u port_to: %u", neterror, strerror, - ip_ntoa(ip, ip_str, sizeof(ip_str)), port_from, port_to); + net_ip_ntoa(ip, &ip_str), port_from, port_to); net_kill_strerror(strerror); kill_networking(temp); @@ -1402,34 +1402,31 @@ void ipport_copy(IP_Port *target, const IP_Port *source) *target = *source; } -/** @brief converts ip into a string +/** @brief Converts IP into a string. * - * @param ip_str must be of length at least IP_NTOA_LEN + * Writes error message into the buffer on error. * - * writes error message into the buffer on error + * @param ip_str contains a buffer of the required size. * - * @return ip_str + * @return Pointer to the buffer inside `ip_str` containing the IP string. */ -const char *ip_ntoa(const IP *ip, char *ip_str, size_t length) +const char *net_ip_ntoa(const IP *ip, Ip_Ntoa *ip_str) { - if (length < IP_NTOA_LEN) { - snprintf(ip_str, length, "Bad buf length"); - return ip_str; - } + assert(ip_str != nullptr); if (ip == nullptr) { - snprintf(ip_str, length, "(IP invalid: NULL)"); - return ip_str; + snprintf(ip_str->buf, sizeof(ip_str->buf), "(IP invalid: NULL)"); + return ip_str->buf; } - if (!ip_parse_addr(ip, ip_str, length)) { - snprintf(ip_str, length, "(IP invalid, family %u)", ip->family.value); - return ip_str; + if (!ip_parse_addr(ip, ip_str->buf, sizeof(ip_str->buf))) { + snprintf(ip_str->buf, sizeof(ip_str->buf), "(IP invalid, family %u)", ip->family.value); + return ip_str->buf; } /* brute force protection against lacking termination */ - ip_str[length - 1] = '\0'; - return ip_str; + ip_str->buf[sizeof(ip_str->buf) - 1] = '\0'; + return ip_str->buf; } bool ip_parse_addr(const IP *ip, char *address, size_t length) @@ -1611,8 +1608,6 @@ bool net_connect(const Logger *log, Socket sock, const IP_Port *ip_port) { struct sockaddr_storage addr = {0}; size_t addrsize; - char ip_str[IP_NTOA_LEN]; - ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str)); if (net_family_is_ipv4(ip_port->ip.family)) { struct sockaddr_in *addr4 = (struct sockaddr_in *)&addr; @@ -1629,15 +1624,18 @@ bool net_connect(const Logger *log, Socket sock, const IP_Port *ip_port) fill_addr6(&ip_port->ip.ip.v6, &addr6->sin6_addr); addr6->sin6_port = ip_port->port; } else { - LOGGER_ERROR(log, "cannot connect to %s:%d which is neither IPv4 nor IPv6", ip_str, ip_port->port); + Ip_Ntoa ip_str; + LOGGER_ERROR(log, "cannot connect to %s:%d which is neither IPv4 nor IPv6", + net_ip_ntoa(&ip_port->ip, &ip_str), ip_port->port); return false; } #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION return addrsize != 0; #else + Ip_Ntoa ip_str; LOGGER_DEBUG(log, "connecting socket %d to %s:%d", - (int)sock.sock, ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str)), net_ntohs(ip_port->port)); + (int)sock.sock, net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port)); errno = 0; if (connect(sock.sock, (struct sockaddr *)&addr, addrsize) == -1) { @@ -1646,7 +1644,8 @@ bool net_connect(const Logger *log, Socket sock, const IP_Port *ip_port) // Non-blocking socket: "Operation in progress" means it's connecting. if (!should_ignore_connect_error(error)) { char *net_strerror = net_new_strerror(error); - LOGGER_ERROR(log, "failed to connect to %s:%d: %d (%s)", ip_str, ip_port->port, error, net_strerror); + LOGGER_ERROR(log, "failed to connect to %s:%d: %d (%s)", + ip_str.buf, ip_port->port, error, net_strerror); net_kill_strerror(net_strerror); return false; } diff --git a/toxcore/network.h b/toxcore/network.h index 5c3149711..b333f1462 100644 --- a/toxcore/network.h +++ b/toxcore/network.h @@ -328,16 +328,21 @@ bool ipv6_ipv4_in_v6(const IP6 *a); /** this would be TOX_INET6_ADDRSTRLEN, but it might be too short for the error message */ #define IP_NTOA_LEN 96 // TODO(irungentoo): magic number. Why not INET6_ADDRSTRLEN ? -/** @brief converts ip into a string + +typedef struct Ip_Ntoa { + char buf[IP_NTOA_LEN]; +} Ip_Ntoa; + +/** @brief Converts IP into a string. * - * @param ip_str must be of length at least IP_NTOA_LEN + * Writes error message into the buffer on error. * - * writes error message into the buffer on error + * @param ip_str contains a buffer of the required size. * - * @return ip_str + * @return Pointer to the buffer inside `ip_str` containing the IP string. */ non_null() -const char *ip_ntoa(const IP *ip, char *ip_str, size_t length); +const char *net_ip_ntoa(const IP *ip, Ip_Ntoa *ip_str); /** * Parses IP structure into an address string. diff --git a/toxcore/network_test.cc b/toxcore/network_test.cc index b620efddb..b9b5c5e26 100644 --- a/toxcore/network_test.cc +++ b/toxcore/network_test.cc @@ -6,48 +6,33 @@ namespace { TEST(IpNtoa, DoesntWriteOutOfBounds) { - char ip_str[IP_NTOA_LEN]; + Ip_Ntoa ip_str; IP ip; ip.family = net_family_ipv6; ip.ip.v6.uint64[0] = -1; ip.ip.v6.uint64[1] = -1; - ip_ntoa(&ip, ip_str, sizeof(ip_str)); + net_ip_ntoa(&ip, &ip_str); - EXPECT_EQ(std::string(ip_str), "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"); - EXPECT_LT(std::string(ip_str).length(), IP_NTOA_LEN); -} - -TEST(IpNtoa, DoesntOverrunSmallBuffer) -{ - // We have to try really hard to trick the compiler here not to realise that - // 10 is too small a buffer for the snprintf inside ip_ntoa and error out. - std::istringstream ss("10"); - size_t len; - ss >> len; - std::vector ip_str(len); - IP *ip = nullptr; - - ip_ntoa(ip, ip_str.data(), ip_str.size()); - - EXPECT_EQ(std::string(ip_str.data()), "Bad buf l"); + EXPECT_EQ(std::string(ip_str.buf), "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"); + EXPECT_LT(std::string(ip_str.buf).length(), IP_NTOA_LEN); } TEST(IpNtoa, ReportsInvalidIpFamily) { - char ip_str[IP_NTOA_LEN]; + Ip_Ntoa ip_str; IP ip; ip.family.value = 255 - net_family_ipv6.value; ip.ip.v4.uint32 = 0; - ip_ntoa(&ip, ip_str, sizeof(ip_str)); + net_ip_ntoa(&ip, &ip_str); - EXPECT_EQ(std::string(ip_str), "(IP invalid, family 245)"); + EXPECT_EQ(std::string(ip_str.buf), "(IP invalid, family 245)"); } TEST(IpNtoa, FormatsIPv4) { - char ip_str[IP_NTOA_LEN]; + Ip_Ntoa ip_str; IP ip; ip.family = net_family_ipv4; ip.ip.v4.uint8[0] = 192; @@ -55,9 +40,9 @@ TEST(IpNtoa, FormatsIPv4) ip.ip.v4.uint8[2] = 0; ip.ip.v4.uint8[3] = 13; - ip_ntoa(&ip, ip_str, sizeof(ip_str)); + net_ip_ntoa(&ip, &ip_str); - EXPECT_EQ(std::string(ip_str), "192.168.0.13"); + EXPECT_EQ(std::string(ip_str.buf), "192.168.0.13"); } TEST(IpParseAddr, FormatsIPv4) diff --git a/toxcore/tox.c b/toxcore/tox.c index 6662cbfee..aeaba94e3 100644 --- a/toxcore/tox.c +++ b/toxcore/tox.c @@ -315,10 +315,9 @@ static void tox_dht_get_nodes_response_handler(const DHT *dht, const Node_format return; } - char ip[IP_NTOA_LEN]; - ip_ntoa(&node->ip_port.ip, ip, sizeof(ip)); - - tox_data->tox->dht_get_nodes_response_callback(tox_data->tox, node->public_key, ip, net_ntohs(node->ip_port.port), + Ip_Ntoa ip_str; + tox_data->tox->dht_get_nodes_response_callback( + tox_data->tox, node->public_key, net_ip_ntoa(&node->ip_port.ip, &ip_str), net_ntohs(node->ip_port.port), tox_data->user_data); }