From 98915c7828dcb006b9149ea1b124438437ec8ddf Mon Sep 17 00:00:00 2001 From: Max Lv Date: Wed, 19 Aug 2026 18:24:33 +0800 Subject: [PATCH] Build as C11 and remove input-sized VLAs from key parsing crypto_parse_key sized a stack array from strlen() of the key given on the command line or in the config file, so a long enough key string overflowed the stack before any validation ran. Only the first key_len bytes were ever used, and base64_decode already stops at the size it is given, so a fixed MAX_KEY_LENGTH buffer is enough. crypto_parse_psk needs one byte of slack to tell an over-long key from an exact-size one. The two "here is a valid key instead" error paths were identical; they are now one helper, which also drops the last input-sized VLA. Switch the standard from gnu99 to C11 via CMAKE_C_STANDARD rather than a hand-written -std flag, so CMake picks the right spelling per compiler. --- CMakeLists.txt | 9 +++++-- src/crypto.c | 67 +++++++++++++++++++++++++++++++------------------- 2 files changed, 49 insertions(+), 27 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b8a108ae..b1217dd5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -13,10 +13,15 @@ include(GNUInstallDirs) # Compilation database for clang-tidy and other tooling set(CMAKE_EXPORT_COMPILE_COMMANDS ON) -# Compiler flags matching autotools +# C11 with GNU extensions. _GNU_SOURCE stays for the glibc-specific +# interfaces used by the Linux-only modules (ss-redir, connmark/nftables). +set(CMAKE_C_STANDARD 11) +set(CMAKE_C_STANDARD_REQUIRED ON) +set(CMAKE_C_EXTENSIONS ON) + # Note: -Werror is applied per-target in src/CMakeLists.txt to avoid # breaking bundled submodules (libcork, libipset, libbloom) -set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99 -D_GNU_SOURCE") +set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -D_GNU_SOURCE") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -O2 -Wall -Wno-deprecated-declarations -fno-strict-aliasing") option(ENABLE_SANITIZERS "Build with AddressSanitizer and UndefinedBehaviorSanitizer" OFF) diff --git a/src/crypto.c b/src/crypto.c index fb0fb3e1..2ff07f89 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -359,15 +359,40 @@ crypto_hkdf_expand(const mbedtls_md_info_t *md, const unsigned char *prk, return 0; } +/* + * Report an unusable key and exit, after showing a freshly generated one of + * the right size so the user has something to paste back. + */ +static void +crypto_key_fatal(const char *what, uint8_t *key, size_t key_len) +{ + char encoded[BASE64_SIZE(MAX_KEY_LENGTH)]; + + rand_bytes(key, key_len); + base64_encode(encoded, sizeof(encoded), key, key_len); + LOGE("Invalid %s for your chosen cipher!", what); + LOGE("It requires a " SIZE_FMT "-byte key encoded with Base64", key_len); + LOGE("Generating a new random key: %s", encoded); + FATAL("Please use the key above or input a valid key"); +} + int crypto_parse_key(const char *base64, uint8_t *key, size_t key_len) { - size_t base64_len = strlen(base64); - int out_len = BASE64_SIZE(base64_len); - uint8_t out[out_len]; + /* + * Decode into a fixed buffer rather than one sized from the input: the + * key comes from the command line or config file, and a VLA sized by + * its length lets a long string overflow the stack. Only the first + * key_len bytes are ever used, and base64_decode never writes past the + * size it is given. + */ + uint8_t out[MAX_KEY_LENGTH]; - out_len = base64_decode(out, base64, out_len); - if (out_len > 0 && out_len >= key_len) { + if (key_len > sizeof(out)) + FATAL("Key length exceeds the maximum supported size"); + + int out_len = base64_decode(out, base64, (int)key_len); + if (out_len > 0 && (size_t)out_len >= key_len) { memcpy(key, out, key_len); #ifdef SS_DEBUG dump("KEY", (char *)key, key_len); @@ -375,14 +400,7 @@ crypto_parse_key(const char *base64, uint8_t *key, size_t key_len) return key_len; } - out_len = BASE64_SIZE(key_len); - char out_key[out_len]; - rand_bytes(key, key_len); - base64_encode(out_key, out_len, key, key_len); - LOGE("Invalid key for your chosen cipher!"); - LOGE("It requires a " SIZE_FMT "-byte key encoded with URL-safe Base64", key_len); - LOGE("Generating a new random key: %s", out_key); - FATAL("Please use the key above or input a valid key"); + crypto_key_fatal("key", key, key_len); return key_len; } @@ -394,22 +412,21 @@ crypto_parse_key(const char *base64, uint8_t *key, size_t key_len) int crypto_parse_psk(const char *base64, uint8_t *key, size_t key_len) { - size_t base64_len = strlen(base64); - int buf_len = BASE64_SIZE(base64_len); - uint8_t out[buf_len]; + /* + * One byte of slack lets an over-long key be told apart from an exact + * one: base64_decode stops at the size it is given, so decoding into + * key_len + 1 bytes yields key_len only for a key of exactly that size. + */ + uint8_t out[MAX_KEY_LENGTH + 1]; - int out_len = base64_decode(out, base64, buf_len); + if (key_len >= sizeof(out)) + FATAL("Key length exceeds the maximum supported size"); + + int out_len = base64_decode(out, base64, (int)key_len + 1); if (out_len != (int)key_len) { - int enc_len = BASE64_SIZE(key_len); - char out_key[enc_len]; - uint8_t rnd[MAX_KEY_LENGTH]; - rand_bytes(rnd, key_len); - base64_encode(out_key, enc_len, rnd, key_len); - LOGE("Invalid pre-shared key for your chosen cipher!"); LOGE("AEAD-2022 requires exactly a " SIZE_FMT "-byte key in base64", key_len); - LOGE("Generating a new random key: %s", out_key); - FATAL("Please use the key above or input a valid key"); + crypto_key_fatal("pre-shared key", key, key_len); } memcpy(key, out, key_len);