mirror of
https://github.com/shadowsocks/shadowsocks-libev.git
synced 2026-10-24 17:33:46 +00:00
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.
This commit is contained in:
+7
-2
@@ -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)
|
||||
|
||||
+42
-25
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user