From b7f95bbe7033a8453986e789e2ce3148426c342e Mon Sep 17 00:00:00 2001 From: iphydf Date: Mon, 21 Feb 2022 22:37:42 +0000 Subject: [PATCH] cleanup: Remove any disallowed casts. The ones in toxav weren't needed. The ones in network.c are non-trivially correct. Now, the code is more easily verifiable. --- .github/scripts/cmake-linux | 1 + .../docker/tox-bootstrapd.sha256 | 2 +- testing/BUILD.bazel | 2 +- toxav/video.c | 2 +- toxcore/ccompat.h | 8 ++- toxcore/network.c | 41 +++++------ toxcore/network_test.cc | 70 +++++++++++++++++++ 7 files changed, 97 insertions(+), 29 deletions(-) diff --git a/.github/scripts/cmake-linux b/.github/scripts/cmake-linux index 73c45f68..2956e711 100755 --- a/.github/scripts/cmake-linux +++ b/.github/scripts/cmake-linux @@ -5,6 +5,7 @@ set -eu NPROC=$(nproc) sudo apt-get install -y --no-install-recommends \ + libgtest-dev \ libmsgpack-dev \ libopus-dev \ libsodium-dev \ diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index 48fe725d..5af8a296 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -e60be1422bb4ba3bf080faca429174afb4804211d5d5fe1ba365ed65f505b361 /usr/local/bin/tox-bootstrapd +65261078cb22504cf84f16d65cbdb0bd3e297bd11f4e801d4127499aab283e96 /usr/local/bin/tox-bootstrapd diff --git a/testing/BUILD.bazel b/testing/BUILD.bazel index ab39e35b..10b2559a 100644 --- a/testing/BUILD.bazel +++ b/testing/BUILD.bazel @@ -27,7 +27,7 @@ sh_test( name = "cimplefmt_test", size = "small", srcs = ["//hs-cimple/tools:cimplefmt"], - args = ["$(locations %s)" % f for f in CIMPLE_FILES], + args = ["--reparse"] + ["$(locations %s)" % f for f in CIMPLE_FILES], data = CIMPLE_FILES, tags = ["haskell"], ) diff --git a/toxav/video.c b/toxav/video.c index 806b320d..a6a3f47f 100644 --- a/toxav/video.c +++ b/toxav/video.c @@ -332,7 +332,7 @@ void vc_iterate(VCSession *vc) dest = vpx_codec_get_frame(vc->decoder, &iter)) { if (vc->vcb != nullptr) { vc->vcb(vc->av, vc->friend_number, dest->d_w, dest->d_h, - (const uint8_t *)dest->planes[0], (const uint8_t *)dest->planes[1], (const uint8_t *)dest->planes[2], + dest->planes[0], dest->planes[1], dest->planes[2], dest->stride[0], dest->stride[1], dest->stride[2], vc->vcb_user_data); } diff --git a/toxcore/ccompat.h b/toxcore/ccompat.h index c144e2e1..3c3c2fe8 100644 --- a/toxcore/ccompat.h +++ b/toxcore/ccompat.h @@ -67,9 +67,13 @@ #if !defined(__cplusplus) || __cplusplus < 201103L #define nullptr NULL #ifndef static_assert +#ifdef __GNUC__ +#define static_assert(cond, msg) extern __attribute__((__unused__)) const int unused_for_static_assert +#else // !__GNUC__ #define static_assert(cond, msg) extern const int unused_for_static_assert -#endif -#endif +#endif // !__GNUC__ +#endif // !static_assert +#endif // !__cplusplus #ifdef __GNUC__ #define GNU_PRINTF(f, a) __attribute__((__format__(__printf__, f, a))) diff --git a/toxcore/network.c b/toxcore/network.c index 95eac358..496402c6 100644 --- a/toxcore/network.c +++ b/toxcore/network.c @@ -1238,27 +1238,14 @@ const char *ip_ntoa(const IP *ip, char *ip_str, size_t length) return ip_str; } - if (ip != nullptr) { - if (net_family_is_ipv4(ip->family)) { - /* returns standard quad-dotted notation */ - struct in_addr addr; - fill_addr4(&ip->ip.v4, &addr); - - ip_str[0] = '\0'; - assert(make_family(ip->family) == AF_INET); - inet_ntop4(&addr, ip_str, length); - } else if (net_family_is_ipv6(ip->family)) { - /* returns hex-groups enclosed into square brackets */ - struct in6_addr addr; - fill_addr6(&ip->ip.v6, &addr); - - assert(make_family(ip->family) == AF_INET6); - inet_ntop6(&addr, ip_str, length); - } else { - snprintf(ip_str, length, "(IP invalid, family %u)", ip->family.value); - } - } else { + if (ip == nullptr) { snprintf(ip_str, length, "(IP invalid: NULL)"); + return ip_str; + } + + if (!ip_parse_addr(ip, ip_str, length)) { + snprintf(ip_str, length, "(IP invalid, family %u)", ip->family.value); + return ip_str; } /* brute force protection against lacking termination */ @@ -1273,15 +1260,21 @@ bool ip_parse_addr(const IP *ip, char *address, size_t length) } if (net_family_is_ipv4(ip->family)) { - const struct in_addr *addr = (const struct in_addr *)&ip->ip.v4; + struct in_addr addr; + static_assert(sizeof(addr) == sizeof(ip->ip.v4.uint32), + "assumption does not hold: in_addr should be 4 bytes"); assert(make_family(ip->family) == AF_INET); - return inet_ntop4(addr, address, length) != nullptr; + fill_addr4(&ip->ip.v4, &addr); + return inet_ntop4(&addr, address, length) != nullptr; } if (net_family_is_ipv6(ip->family)) { - const struct in6_addr *addr = (const struct in6_addr *)&ip->ip.v6; + struct in6_addr addr; + static_assert(sizeof(addr) == sizeof(ip->ip.v6.uint8), + "assumption does not hold: in6_addr should be 16 bytes"); assert(make_family(ip->family) == AF_INET6); - return inet_ntop6(addr, address, length) != nullptr; + fill_addr6(&ip->ip.v6, &addr); + return inet_ntop6(&addr, address, length) != nullptr; } return false; diff --git a/toxcore/network_test.cc b/toxcore/network_test.cc index 8323cce0..b620efdd 100644 --- a/toxcore/network_test.cc +++ b/toxcore/network_test.cc @@ -18,4 +18,74 @@ TEST(IpNtoa, DoesntWriteOutOfBounds) 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"); +} + +TEST(IpNtoa, ReportsInvalidIpFamily) +{ + char ip_str[IP_NTOA_LEN]; + IP ip; + ip.family.value = 255 - net_family_ipv6.value; + ip.ip.v4.uint32 = 0; + + ip_ntoa(&ip, ip_str, sizeof(ip_str)); + + EXPECT_EQ(std::string(ip_str), "(IP invalid, family 245)"); +} + +TEST(IpNtoa, FormatsIPv4) +{ + char ip_str[IP_NTOA_LEN]; + IP ip; + ip.family = net_family_ipv4; + ip.ip.v4.uint8[0] = 192; + ip.ip.v4.uint8[1] = 168; + ip.ip.v4.uint8[2] = 0; + ip.ip.v4.uint8[3] = 13; + + ip_ntoa(&ip, ip_str, sizeof(ip_str)); + + EXPECT_EQ(std::string(ip_str), "192.168.0.13"); +} + +TEST(IpParseAddr, FormatsIPv4) +{ + char ip_str[IP_NTOA_LEN]; + IP ip; + ip.family = net_family_ipv4; + ip.ip.v4.uint8[0] = 192; + ip.ip.v4.uint8[1] = 168; + ip.ip.v4.uint8[2] = 0; + ip.ip.v4.uint8[3] = 13; + + ip_parse_addr(&ip, ip_str, sizeof(ip_str)); + + EXPECT_EQ(std::string(ip_str), "192.168.0.13"); +} + +TEST(IpParseAddr, FormatsIPv6) +{ + char ip_str[IP_NTOA_LEN]; + IP ip; + ip.family = net_family_ipv6; + ip.ip.v6.uint64[0] = -1; + ip.ip.v6.uint64[1] = -1; + + ip_parse_addr(&ip, ip_str, sizeof(ip_str)); + + EXPECT_EQ(std::string(ip_str), "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"); +} + } // namespace