diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index 6fb81bb..bc08d57 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -12,7 +12,7 @@ jobs: container: gitea.tap-tap.win/taptap/fastsync-ci:v10 steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: clang-format check run: find src/ tests/ -name '*.c' -o -name '*.h' | xargs clang-format --dry-run --Werror @@ -30,7 +30,7 @@ jobs: needs: lint steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Configure run: cmake -B build -S . -DSTRICT_WARNINGS=ON @@ -59,7 +59,7 @@ jobs: sanitizer: [address, undefined] steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Configure run: cmake -B build-${{ matrix.sanitizer }} -S . -DSANITIZER=${{ matrix.sanitizer }} @@ -77,7 +77,7 @@ jobs: if: github.event_name == 'push' steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Configure (clang + fuzz) run: CC=clang CXX=clang++ cmake -B build-fuzz -S . -DENABLE_FUZZ=ON @@ -99,7 +99,7 @@ jobs: if: github.event_name == 'push' steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Configure run: cmake -B build -S . -DENABLE_COVERAGE=ON @@ -123,7 +123,7 @@ jobs: if: github.event_name == 'push' steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Configure run: cmake -B build -S . -DSTRICT_WARNINGS=ON diff --git a/CMakeLists.txt b/CMakeLists.txt index 3ec3b24..ff92672 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -38,11 +38,24 @@ if(ENABLE_COVERAGE) add_link_options(--coverage) endif() +# --- Build hardening option --- +# Production hardening is applied to the shipping server/client binaries only, +# and only when no sanitizer or coverage instrumentation is active: sanitizers +# carry their own instrumentation, and _FORTIFY_SOURCE requires an optimising +# build (never the -O0 used for coverage). +option(ENABLE_HARDENING "Enable compiler/linker hardening for production targets" ON) +set(HARDENING_ACTIVE OFF) +if(ENABLE_HARDENING AND SANITIZER STREQUAL "none" AND NOT ENABLE_COVERAGE) + set(HARDENING_ACTIVE ON) +endif() + include(FetchContent) FetchContent_Declare( xxhash GIT_REPOSITORY https://github.com/Cyan4973/xxHash - GIT_TAG v0.8.3 + # v0.8.3 is a lightweight tag pointing at this exact commit (no ^{} peel + # entry); pin the commit SHA instead of the mutable tag. + GIT_TAG e626a72bc2321cd320e953a0ccf1584cad60f363 # v0.8.3 SOURCE_SUBDIR cmake_unofficial ) FetchContent_MakeAvailable(xxhash) @@ -73,6 +86,33 @@ add_executable(client ${CLIENT_SRCS} ${SHARED_SRCS} ${FILE_STORE_SRCS} ${SERVER_ target_include_directories(client PRIVATE src/shared src/server src/client) target_link_libraries(client PRIVATE Threads::Threads ${ZSTD_LIBRARY} OpenSSL::SSL OpenSSL::Crypto xxhash) +# --- Production hardening --- +# Each compile flag is probed so a compiler/architecture that lacks it still +# configures cleanly. _FORTIFY_SOURCE is guarded separately because it only +# works in an optimising build. xxHash is a static archive built by +# FetchContent, so it must be position-independent for the -pie link. +if(HARDENING_ACTIVE) + set_target_properties(xxhash PROPERTIES POSITION_INDEPENDENT_CODE ON) + include(CheckCCompilerFlag) + foreach(flag -fstack-protector-strong -fstack-clash-protection -fPIE) + string(MAKE_C_IDENTIFIER "HARDEN_${flag}" _harden_var) + check_c_compiler_flag("${flag}" ${_harden_var}) + endforeach() + check_c_compiler_flag("-D_FORTIFY_SOURCE=2" HARDEN_FORTIFY_SOURCE) + foreach(target server client) + foreach(flag -fstack-protector-strong -fstack-clash-protection -fPIE) + string(MAKE_C_IDENTIFIER "HARDEN_${flag}" _harden_var) + if(${_harden_var}) + target_compile_options(${target} PRIVATE ${flag}) + endif() + endforeach() + if(HARDEN_FORTIFY_SOURCE) + target_compile_options(${target} PRIVATE -D_FORTIFY_SOURCE=2) + endif() + target_link_options(${target} PRIVATE -pie -Wl,-z,relro -Wl,-z,now -Wl,-z,noexecstack) + endforeach() +endif() + # --- Testing --- enable_testing() diff --git a/src/shared/array_list.c b/src/shared/array_list.c index 95799b0..7813e6d 100644 --- a/src/shared/array_list.c +++ b/src/shared/array_list.c @@ -1,6 +1,7 @@ #include "log.h" #include "array_list.h" #include "protocol.h" +#include #include #include #include @@ -39,6 +40,8 @@ void array_list_delete(ArrayList* array_list) { static bool array_list_extend(ArrayList* array_list) { if (array_list == NULL) return false; + if (array_list->capacity > INT_MAX / 2) + return false; int new_capacity = array_list->capacity * 2; if (new_capacity == 0) new_capacity = INITIAL_ARRAY_SIZE; diff --git a/src/shared/daemon_conf.c b/src/shared/daemon_conf.c index e806549..7a1c83d 100644 --- a/src/shared/daemon_conf.c +++ b/src/shared/daemon_conf.c @@ -1,4 +1,5 @@ #include "daemon_conf.h" +#include "credentials.h" #include "utils.h" #include #include @@ -192,6 +193,12 @@ static bool apply_module_key(DaemonModule* module, char* key, char* value, char* const char* user = trim_ws(token); if (*user == '\0') continue; + if (!credentials_username_valid(user)) { + set_error(err, err_size, "module '%s': invalid 'auth users' entry '%s'", module->name, + user); + free(list); + return false; + } char** grown = realloc(module->auth_users, (size_t)(module->auth_user_count + 1) * sizeof(char*)); if (!grown) { diff --git a/src/shared/file_list.c b/src/shared/file_list.c index 8e40c78..528c8d4 100644 --- a/src/shared/file_list.c +++ b/src/shared/file_list.c @@ -2,6 +2,7 @@ #include "log.h" #include "utils.h" #include +#include #include #include #include @@ -51,7 +52,8 @@ static int normalize_entry(const char* raw, size_t len, bool strip_line_endings, if (len == 0) return 0; if (raw[0] == '/') { - snprintf(err, err_size, "absolute path entries are not allowed: '%.*s'", (int)len, raw); + int print_len = len > (size_t)INT_MAX ? INT_MAX : (int)len; + snprintf(err, err_size, "absolute path entries are not allowed: '%.*s'", print_len, raw); return -1; } /* Reject NUL bytes inside a token defensively (NUL-delimited mode splits on diff --git a/src/shared/transport_ssh.c b/src/shared/transport_ssh.c index b5010ac..97eb1ea 100644 --- a/src/shared/transport_ssh.c +++ b/src/shared/transport_ssh.c @@ -128,7 +128,7 @@ char* ssh_build_remote_command(const char* server_path, bool old_args, char* con q++; len++; } - if (len > SIZE_MAX - q * 3 || len + q * 3 + 3 > SIZE_MAX - command_len) + if (q > (SIZE_MAX - len) / 3 || len + q * 3 + 3 > SIZE_MAX - command_len) return NULL; command_len += len + q * 3 + 3; } diff --git a/src/shared/transport_tls.c b/src/shared/transport_tls.c index 85bb5a1..81aa483 100644 --- a/src/shared/transport_tls.c +++ b/src/shared/transport_tls.c @@ -65,6 +65,18 @@ static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key SSL_CTX_free(ctx); return NULL; } + /* TLS 1.3 ciphersuites are configured separately from the TLS 1.2 and below + * cipher list above. Pin the three AEAD suites OpenSSL offers, dropping + * TLS_AES_128_CCM_SHA256 and the CCM_8 variant, and fail closed if the + * library rejects the policy. SSL_CTX_set_ciphersuites needs OpenSSL 1.1.1; + * earlier versions have no TLS 1.3, so the call is compile-guarded. */ +#if OPENSSL_VERSION_NUMBER >= 0x10101000L + if (SSL_CTX_set_ciphersuites( + ctx, "TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256:TLS_AES_128_GCM_SHA256") != 1) { + SSL_CTX_free(ctx); + return NULL; + } +#endif if (cert && key) { struct stat key_stat; diff --git a/tests/test_array_list.c b/tests/test_array_list.c index 964458d..a917dae 100644 --- a/tests/test_array_list.c +++ b/tests/test_array_list.c @@ -1,6 +1,7 @@ #include "test_array_list.h" #include "array_list.h" #include "test_utils.h" +#include #include static int destroyer_calls = 0; @@ -9,7 +10,7 @@ static void test_destroyer(void* item) { free(item); } -void test_array_list() { +static void test_array_list_basic() { ArrayList* list = array_list_create(free); EXPECT_NOT_NULL(list); EXPECT_EQ_INT(list->size, 0); @@ -54,3 +55,20 @@ void test_array_list() { array_list_delete(list); EXPECT_EQ_INT(destroyer_calls, 106); } + +/* A capacity that would overflow `capacity * 2` must be refused instead of + * wrapping into signed-overflow UB; array_list_add surfaces the failure. */ +static void test_array_list_extend_overflow_guard() { + ArrayList* list = array_list_create(NULL); + EXPECT_NOT_NULL(list); + list->capacity = INT_MAX / 2 + 1; + list->size = list->capacity; + EXPECT_FALSE(array_list_add(list, NULL)); + list->size = 0; + array_list_delete(list); +} + +void test_array_list() { + test_array_list_basic(); + test_array_list_extend_overflow_guard(); +} diff --git a/tests/test_daemon_conf.c b/tests/test_daemon_conf.c index 9580f8b..1f54a2e 100644 --- a/tests/test_daemon_conf.c +++ b/tests/test_daemon_conf.c @@ -1,4 +1,5 @@ #include "test_daemon_conf.h" +#include "credentials.h" #include "daemon_conf.h" #include "test_utils.h" #include @@ -320,6 +321,51 @@ static void test_daemon_conf_dparam_override() { daemon_conf_free(conf); } +/* Each `auth users` entry is validated with the same username rule as the + * credential store, so invisible whitespace/control characters can never make + * an exact strcmp match ambiguous. */ +static void test_daemon_conf_auth_users_validated() { + char* path; + char err[256]; + const DaemonConf* conf; + + EXPECT_EQ_INT(write_conf("[m]\npath = /x\nauth users = alice, bad user\n", &path), 0); + conf = daemon_conf_load(path, err, sizeof(err)); + free(path); + EXPECT_NULL(conf); + EXPECT_TRUE(strstr(err, "invalid 'auth users' entry") != NULL); + + EXPECT_EQ_INT(write_conf("[m]\npath = /x\nauth users = good\tbad\n", &path), 0); + conf = daemon_conf_load(path, err, sizeof(err)); + free(path); + EXPECT_NULL(conf); + EXPECT_TRUE(strstr(err, "invalid 'auth users' entry") != NULL); + + /* An over-long name exceeds CREDENTIAL_MAX_USER_LEN and is rejected. */ + { + char body[CREDENTIAL_MAX_USER_LEN + 128]; + int n = snprintf(body, sizeof(body), "[m]\npath = /x\nauth users = "); + memset(body + n, 'a', CREDENTIAL_MAX_USER_LEN + 1); + body[n + CREDENTIAL_MAX_USER_LEN + 1] = '\n'; + body[n + CREDENTIAL_MAX_USER_LEN + 2] = '\0'; + EXPECT_EQ_INT(write_conf(body, &path), 0); + conf = daemon_conf_load(path, err, sizeof(err)); + free(path); + EXPECT_NULL(conf); + EXPECT_TRUE(strstr(err, "invalid 'auth users' entry") != NULL); + } + + /* Empty entries between commas are skipped, not treated as invalid. */ + EXPECT_EQ_INT(write_conf("[m]\npath = /x\nauth users = alice,, bob\n", &path), 0); + DaemonConf* ok_conf = daemon_conf_load(path, err, sizeof(err)); + free(path); + EXPECT_NOT_NULL(ok_conf); + EXPECT_EQ_INT(ok_conf->modules[0].auth_user_count, 2); + EXPECT_EQ_STR(ok_conf->modules[0].auth_users[0], "alice"); + EXPECT_EQ_STR(ok_conf->modules[0].auth_users[1], "bob"); + daemon_conf_free(ok_conf); +} + static void test_daemon_module_name_valid() { EXPECT_TRUE(daemon_module_name_valid("backup")); EXPECT_TRUE(daemon_module_name_valid("Backup_2")); @@ -351,5 +397,6 @@ void test_daemon_conf() { test_daemon_conf_missing_file_rejected(); test_daemon_conf_find_module(); test_daemon_conf_dparam_override(); + test_daemon_conf_auth_users_validated(); test_daemon_module_name_valid(); } \ No newline at end of file