test: Add a simple test for ip_ntoa.

Just to demonstrate that it will never exceed its input buffer.

.circleci/config.yml

@@ -9,8 +9,9 @@ workflows:
       - asan
       - tsan
       # Static analysis
-      - static-analysis
+      - clang-tidy
       - infer
+      - static-analysis
 
 jobs:
   asan:
@@ -89,3 +90,24 @@ jobs:
       - run: other/analysis/run-clang-analyze
       - run: other/analysis/run-cppcheck
       - run: other/analysis/run-gcc
+
+  clang-tidy:
+    working_directory: ~/work
+    docker:
+      - image: ubuntu
+
+    steps:
+      - checkout
+      - run:
+          apt-get update &&
+          DEBIAN_FRONTEND=noninteractive
+          apt-get install -y --no-install-recommends
+            build-essential
+            clang-tidy-11
+            cmake
+            libconfig-dev
+            libopus-dev
+            libsodium-dev
+            libvpx-dev
+      - run: cmake . -B_build -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
+      - run: other/analysis/run-clang-tidy

.github/settings.yml

@@ -15,6 +15,7 @@ branches:
           - Codacy Static Code Analysis
           - CodeFactor
           - "ci/circleci: asan"
+          - "ci/circleci: clang-tidy"
           - "ci/circleci: infer"
           - "ci/circleci: static-analysis"
           - "ci/circleci: tsan"

.github/workflows/clang-tidy-review.yml

@@ -1,28 +0,0 @@
-name: clang-tidy-review
-
-on:
-  pull_request:
-    branches: [master]
-
-jobs:
-  build:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v2
-      - name: Install compiledb
-        run: pip install compiledb
-      - name: Install dependencies
-        run:
-          sudo apt-get install -y --no-install-recommends
-            cmake
-            libconfig-dev
-            libopus-dev
-            libsodium-dev
-            libvpx-dev
-            pkg-config
-      - name: Generate compile_commands.json
-        run:
-          cmake . && compiledb make -j4
-
-      - uses: ZedThree/clang-tidy-review@v0.7.0
-        id: review

build/Makefile.am

@@ -24,3 +24,5 @@ include ../other/Makefile.inc
 include ../testing/Makefile.inc
 include ../other/bootstrap_daemon/src/Makefile.inc
 include ../auto_tests/Makefile.inc
+
+build-tests: $(check_PROGRAMS)

other/analysis/run-clang-tidy

@@ -0,0 +1,62 @@
+#!/bin/sh
+
+CHECKS="*"
+CHECKS="$CHECKS,-bugprone-not-null-terminated-result"
+CHECKS="$CHECKS,-bugprone-reserved-identifier"
+CHECKS="$CHECKS,-bugprone-sizeof-expression"
+CHECKS="$CHECKS,-cert-dcl37-c"
+CHECKS="$CHECKS,-cert-dcl51-cpp"
+CHECKS="$CHECKS,-clang-analyzer-optin.performance.Padding"
+CHECKS="$CHECKS,-cppcoreguidelines-avoid-magic-numbers"
+CHECKS="$CHECKS,-cppcoreguidelines-init-variables"
+CHECKS="$CHECKS,-hicpp-multiway-paths-covered"
+CHECKS="$CHECKS,-hicpp-signed-bitwise"
+CHECKS="$CHECKS,-llvm-else-after-return"
+CHECKS="$CHECKS,-llvmlibc-restrict-system-libc-headers"
+CHECKS="$CHECKS,-misc-redundant-expression"
+CHECKS="$CHECKS,-misc-unused-parameters"
+CHECKS="$CHECKS,-readability-else-after-return"
+CHECKS="$CHECKS,-readability-inconsistent-declaration-parameter-name"
+CHECKS="$CHECKS,-readability-magic-numbers"
+
+ERRORS="*"
+ERRORS="$ERRORS,-bugprone-branch-clone"
+ERRORS="$ERRORS,-bugprone-integer-division"
+ERRORS="$ERRORS,-bugprone-narrowing-conversions"
+ERRORS="$ERRORS,-clang-analyzer-core.NonNullParamChecker"
+ERRORS="$ERRORS,-clang-analyzer-core.NullDereference"
+ERRORS="$ERRORS,-clang-analyzer-optin.portability.UnixAPI"
+ERRORS="$ERRORS,-clang-analyzer-valist.Uninitialized"
+ERRORS="$ERRORS,-cppcoreguidelines-avoid-non-const-global-variables"
+ERRORS="$ERRORS,-cppcoreguidelines-narrowing-conversions"
+ERRORS="$ERRORS,-google-readability-casting"
+ERRORS="$ERRORS,-misc-no-recursion"
+ERRORS="$ERRORS,-readability-redundant-control-flow"
+
+# TODO(iphydf): Fix these.
+ERRORS="$ERRORS,-android-cloexec-accept"
+ERRORS="$ERRORS,-android-cloexec-fopen"
+ERRORS="$ERRORS,-bugprone-macro-parentheses"
+ERRORS="$ERRORS,-bugprone-posix-return"
+ERRORS="$ERRORS,-bugprone-signed-char-misuse"
+ERRORS="$ERRORS,-cert-err34-c"
+ERRORS="$ERRORS,-cert-str34-c"
+ERRORS="$ERRORS,-clang-analyzer-security.insecureAPI.strcpy"
+ERRORS="$ERRORS,-hicpp-uppercase-literal-suffix"
+ERRORS="$ERRORS,-readability-non-const-parameter"
+ERRORS="$ERRORS,-readability-uppercase-literal-suffix"
+
+set -eux
+
+clang-tidy-11 \
+  -p=_build \
+  --extra-arg=-DMIN_LOGGER_LEVEL=LOGGER_LEVEL_TRACE \
+  --checks="$CHECKS" \
+  --warnings-as-errors="$ERRORS" \
+  --use-color \
+  other/bootstrap_daemon/src/*.c \
+  other/*.c \
+  toxav/*.c \
+  toxcore/*.c \
+  toxencryptsave/*.c \
+  "$@"

other/bootstrap_daemon/src/tox-bootstrapd.c

@@ -12,9 +12,9 @@
 #endif
 
 // system provided
+#include <signal.h> // system header, rather than C, because we need it for POSIX sigaction(2)
 #include <sys/resource.h>
 #include <sys/stat.h>
-#include <signal.h> // system header, rather than C, because we need it for POSIX sigaction(2)
 #include <unistd.h>
 
 // C

toxcore/BUILD.bazel

@@ -115,6 +115,16 @@ cc_library(
     ],
 )
 
+cc_test(
+    name = "network_test",
+    size = "small",
+    srcs = ["network_test.cc"],
+    deps = [
+        ":network",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_test(
     name = "util_test",
     size = "small",

toxcore/ccompat.h

@@ -43,7 +43,7 @@
 #if !defined(__cplusplus) || __cplusplus < 201103L
 #define nullptr NULL
 #ifndef static_assert
-#define static_assert(cond, msg) extern int unused_for_static_assert
+#define static_assert(cond, msg) extern const int unused_for_static_assert
 #endif
 #endif
 

toxcore/network.c

@@ -1129,7 +1129,7 @@ const char *ip_ntoa(const IP *ip, char *ip_str, size_t length)
             struct in_addr addr;
             fill_addr4(ip->ip.v4, &addr);
 
-            ip_str[0] = 0;
+            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)) {
@@ -1142,7 +1142,7 @@ const char *ip_ntoa(const IP *ip, char *ip_str, size_t length)
             inet_ntop6(&addr, &ip_str[1], length - 3);
             const size_t len = strlen(ip_str);
             ip_str[len] = ']';
-            ip_str[len + 1] = 0;
+            ip_str[len + 1] = '\0';
         } else {
             snprintf(ip_str, length, "(IP invalid, family %u)", ip->family.value);
         }
@@ -1151,7 +1151,7 @@ const char *ip_ntoa(const IP *ip, char *ip_str, size_t length)
     }
 
     /* brute force protection against lacking termination */
-    ip_str[length - 1] = 0;
+    ip_str[length - 1] = '\0';
     return ip_str;
 }
 

toxcore/network_test.cc

@@ -0,0 +1,20 @@
+#include "network.h"
+
+#include <gtest/gtest.h>
+
+namespace {
+
+TEST(IpNtoa, DoesntWriteOutOfBounds) {
+  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_ntoa(&ip, ip_str, sizeof(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);
+}
+
+}  // namespace