From a0fbaf0889fda3788943baec80671ecc870a2925 Mon Sep 17 00:00:00 2001 From: Kirill Isakov Date: Thu, 28 Apr 2022 18:40:54 +0600 Subject: [PATCH] Use hardening option to add only hardening flags Compiler flags that enable warnings were hidden behind the 'hardening' option. This was a direct port of the previous autoconf config, but it probably makes sense to always show them as they have no effect on compiler's output (unlike hardening flags which distributors might want to alter or disable completely). Also remove -fwrapv (which is enabled by -fno-strict-overflow) and fix all new warnings we've added (like VLAs in libgcrypt code, so it should now be buildable with MSVC in case anyone wishes to do it). --- meson.build | 51 ++++++++++++++++++++++++++++++--------------- src/compression.h | 2 -- src/fsck.c | 1 + src/gcrypt/cipher.c | 2 +- src/gcrypt/digest.c | 4 ++-- src/gcrypt/pem.c | 2 +- src/gcrypt/prf.c | 18 +++++++++------- src/gcrypt/rsa.c | 12 +++++++---- src/gcrypt/rsa.h | 5 +++++ src/gcrypt/rsagen.c | 2 +- src/ifconfig.c | 7 +++++++ src/logger.c | 1 + src/net_packet.c | 1 + src/protocol_auth.c | 1 + src/script.h | 4 ++-- src/sptps.c | 10 +++++---- src/sptps.h | 6 +++--- src/sptps_test.c | 7 +++---- src/tincctl.c | 14 ++++++------- src/tincctl.h | 2 +- src/tincd.c | 14 ++++++------- src/xalloc.h | 1 + 22 files changed, 102 insertions(+), 65 deletions(-) diff --git a/meson.build b/meson.build index 85d16122..43ae9744 100644 --- a/meson.build +++ b/meson.build @@ -50,6 +50,34 @@ endif cc_flags = [cc_defs] ld_flags = [] +if cc_name != 'msvc' + cc_flags += [ + '-Qunused-arguments', + '-Wbad-function-cast', + '-Wduplicated-branches', + '-Wduplicated-cond', + '-Wformat-overflow=2', + '-Wformat-truncation=1', # 2 prints too much noise + '-Wformat=2', + '-Wlogical-op', + '-Wmissing-declarations', + '-Wmissing-noreturn', + '-Wmissing-prototypes', + '-Wno-embedded-directive', + '-Wold-style-definition', + '-Wredundant-decls', + '-Wreturn-type', + '-Wstrict-prototypes', + '-Wswitch-enum', + '-Wtrampolines', # may require executable stack which is disabled + '-Wvla', # VLAs are not supported by MSVC + '-Wwrite-strings', + '-fdiagnostics-show-option', + '-fno-strict-overflow', + '-fstrict-aliasing', + ] +endif + if opt_static.auto() static = os_name == 'windows' else @@ -74,26 +102,15 @@ if opt_harden else cc_flags += [ '-D_FORTIFY_SOURCE=2', - '-fwrapv', - '-fno-strict-overflow', - '-Wreturn-type', - '-Wold-style-definition', - '-Wmissing-declarations', - '-Wmissing-prototypes', - '-Wstrict-prototypes', - '-Wredundant-decls', - '-Wbad-function-cast', - '-Wwrite-strings', - '-fdiagnostics-show-option', - '-fstrict-aliasing', - '-Wmissing-noreturn', + '-fcf-protection=full', + '-fstack-protector-strong', ] - if cc_name == 'clang' - cc_flags += '-Qunused-arguments' - endif - ld_flags += ['-Wl,-z,relro', '-Wl,-z,now'] + ld_flags += ['-Wl,-z,relro', '-Wl,-z,now', '-Wl,-z,noexecstack'] if os_name == 'windows' ld_flags += ['-Wl,--dynamicbase', '-Wl,--nxcompat'] + else + # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90458 + cc_flags += '-fstack-clash-protection' endif endif endif diff --git a/src/compression.h b/src/compression.h index 98eca331..7fe5bb65 100644 --- a/src/compression.h +++ b/src/compression.h @@ -18,8 +18,6 @@ typedef enum compression_level_t { COMPRESS_LZO_HI = 11, COMPRESS_LZ4 = 12, - - COMPRESS_GUARD = INT_MAX, /* ensure that sizeof(compression_level_t) == sizeof(int) */ } compression_level_t; STATIC_ASSERT(sizeof(compression_level_t) == sizeof(int), "compression_level_t has invalid size"); diff --git a/src/fsck.c b/src/fsck.c index 8d818a3c..70ca84d6 100644 --- a/src/fsck.c +++ b/src/fsck.c @@ -64,6 +64,7 @@ again: goto again; } +static void print_tinc_cmd(const char *format, ...) ATTR_FORMAT(printf, 1, 2); static void print_tinc_cmd(const char *format, ...) { if(confbasegiven) { fprintf(stderr, "%s -c %s ", exe_name, confbase); diff --git a/src/gcrypt/cipher.c b/src/gcrypt/cipher.c index 2813da6b..ee3856ad 100644 --- a/src/gcrypt/cipher.c +++ b/src/gcrypt/cipher.c @@ -208,7 +208,7 @@ bool cipher_set_key_from_rsa(cipher_t *cipher, void *key, size_t len, bool encry bool cipher_encrypt(cipher_t *cipher, const void *indata, size_t inlen, void *outdata, size_t *outlen, bool oneshot) { gcry_error_t err; - uint8_t pad[cipher->blklen]; + uint8_t *pad = alloca(cipher->blklen); if(cipher->padding) { if(!oneshot) { diff --git a/src/gcrypt/digest.c b/src/gcrypt/digest.c index cfea1f0a..e0291689 100644 --- a/src/gcrypt/digest.c +++ b/src/gcrypt/digest.c @@ -149,7 +149,7 @@ bool digest_create(digest_t *digest, const void *indata, size_t inlen, void *out memcpy(outdata, tmpdata, digest->maclength); } else { - char tmpdata[len]; + char *tmpdata = alloca(len); gcry_md_hash_buffer(digest->algo, tmpdata, indata, inlen); memcpy(outdata, tmpdata, digest->maclength); } @@ -159,7 +159,7 @@ bool digest_create(digest_t *digest, const void *indata, size_t inlen, void *out bool digest_verify(digest_t *digest, const void *indata, size_t inlen, const void *cmpdata) { size_t len = digest->maclength; - uint8_t outdata[len]; + uint8_t *outdata = alloca(len); return digest_create(digest, indata, inlen, outdata) && !memcmp(cmpdata, outdata, len); } diff --git a/src/gcrypt/pem.c b/src/gcrypt/pem.c index 0f7763c5..56858f23 100644 --- a/src/gcrypt/pem.c +++ b/src/gcrypt/pem.c @@ -93,7 +93,7 @@ bool pem_encode(FILE *fp, const char *header, uint8_t *buf, size_t size) { return false; } - char b64[B64_SIZE(size)]; + char *b64 = alloca(B64_SIZE(size)); const size_t b64len = b64encode(b64, buf, size); for(size_t i = 0; i < b64len; i += 64) { diff --git a/src/gcrypt/prf.c b/src/gcrypt/prf.c index 1e5bb9f9..d11ef777 100644 --- a/src/gcrypt/prf.c +++ b/src/gcrypt/prf.c @@ -32,7 +32,9 @@ static const size_t mdlen = 64; static const size_t blklen = 128; static bool hmac_sha512(const uint8_t *key, size_t keylen, const uint8_t *msg, size_t msglen, uint8_t *out) { - uint8_t tmp[blklen + mdlen]; + const size_t tmplen = blklen + mdlen; + uint8_t *tmp = alloca(tmplen); + sha512_context md; if(keylen <= blklen) { @@ -69,7 +71,7 @@ static bool hmac_sha512(const uint8_t *key, size_t keylen, const uint8_t *msg, s // opad memxor(tmp, 0x36 ^ 0x5c, blklen); - if(sha512(tmp, sizeof(tmp), out) != 0) { + if(sha512(tmp, tmplen, out) != 0) { return false; } @@ -86,28 +88,30 @@ bool prf(const uint8_t *secret, size_t secretlen, uint8_t *seed, size_t seedlen, It consists of the previous HMAC result plus the seed. */ - uint8_t data[mdlen + seedlen]; + const size_t datalen = mdlen + seedlen; + uint8_t *data = alloca(datalen); + memset(data, 0, mdlen); memcpy(data + mdlen, seed, seedlen); - uint8_t hash[mdlen]; + uint8_t *hash = alloca(mdlen); while(outlen > 0) { /* Inner HMAC */ - if(!hmac_sha512(secret, secretlen, data, sizeof(data), data)) { + if(!hmac_sha512(secret, secretlen, data, datalen, data)) { return false; } /* Outer HMAC */ if(outlen >= mdlen) { - if(!hmac_sha512(secret, secretlen, data, sizeof(data), out)) { + if(!hmac_sha512(secret, secretlen, data, datalen, out)) { return false; } out += mdlen; outlen -= mdlen; } else { - if(!hmac_sha512(secret, secretlen, data, sizeof(data), hash)) { + if(!hmac_sha512(secret, secretlen, data, datalen, hash)) { return false; } diff --git a/src/gcrypt/rsa.c b/src/gcrypt/rsa.c index 04aa358d..292c7739 100644 --- a/src/gcrypt/rsa.c +++ b/src/gcrypt/rsa.c @@ -111,8 +111,12 @@ static bool ber_read_mpi(unsigned char **p, size_t *buflen, gcry_mpi_t *mpi) { return mpi ? !err : true; } +rsa_t *rsa_new(void) { + return xzalloc(sizeof(rsa_t)); +} + rsa_t *rsa_set_hex_public_key(const char *n, const char *e) { - rsa_t *rsa = xzalloc(sizeof(rsa_t)); + rsa_t *rsa = rsa_new(); gcry_error_t err = gcry_mpi_scan(&rsa->n, GCRYMPI_FMT_HEX, n, 0, NULL); @@ -130,7 +134,7 @@ rsa_t *rsa_set_hex_public_key(const char *n, const char *e) { } rsa_t *rsa_set_hex_private_key(const char *n, const char *e, const char *d) { - rsa_t *rsa = xzalloc(sizeof(rsa_t)); + rsa_t *rsa = rsa_new(); gcry_error_t err = gcry_mpi_scan(&rsa->n, GCRYMPI_FMT_HEX, n, 0, NULL); @@ -162,7 +166,7 @@ rsa_t *rsa_read_pem_public_key(FILE *fp) { return NULL; } - rsa_t *rsa = xzalloc(sizeof(rsa_t)); + rsa_t *rsa = rsa_new(); if(!ber_skip_sequence(&derp, &derlen) || !ber_read_mpi(&derp, &derlen, &rsa->n) @@ -185,7 +189,7 @@ rsa_t *rsa_read_pem_private_key(FILE *fp) { return NULL; } - rsa_t *rsa = xzalloc(sizeof(rsa_t)); + rsa_t *rsa = rsa_new(); if(!ber_skip_sequence(&derp, &derlen) || !ber_read_mpi(&derp, &derlen, NULL) diff --git a/src/gcrypt/rsa.h b/src/gcrypt/rsa.h index e6bb5ec9..14f03818 100644 --- a/src/gcrypt/rsa.h +++ b/src/gcrypt/rsa.h @@ -22,6 +22,9 @@ #include +#include "../rsa.h" +#include "../xalloc.h" + #define TINC_RSA_INTERNAL typedef struct rsa { gcry_mpi_t n; @@ -29,4 +32,6 @@ typedef struct rsa { gcry_mpi_t d; } rsa_t; +extern rsa_t *rsa_new(void) ATTR_MALLOC ATTR_DEALLOCATOR(rsa_free); + #endif diff --git a/src/gcrypt/rsagen.c b/src/gcrypt/rsagen.c index b89225d4..3469c336 100644 --- a/src/gcrypt/rsagen.c +++ b/src/gcrypt/rsagen.c @@ -297,7 +297,7 @@ rsa_t *rsa_generate(size_t bits, unsigned long exponent) { return NULL; } - rsa_t *rsa = xzalloc(sizeof(*rsa)); + rsa_t *rsa = rsa_new(); rsa->n = find_mpi(s_rsa, "n"); rsa->e = find_mpi(s_rsa, "e"); diff --git a/src/ifconfig.c b/src/ifconfig.c index f4ce313a..d4f13865 100644 --- a/src/ifconfig.c +++ b/src/ifconfig.c @@ -105,6 +105,7 @@ void ifconfig_address(FILE *out, const char *value) { ipv6 = address; break; + case SUBNET_MAC: default: return; } @@ -208,6 +209,7 @@ void ifconfig_route(FILE *out, const char *value) { fprintf(out, "ip route add %s via %s dev \"$INTERFACE\" onlink\n", subnet_str, gateway_str); break; + case SUBNET_MAC: default: return; } @@ -221,6 +223,7 @@ void ifconfig_route(FILE *out, const char *value) { fprintf(out, "ip route add %s dev \"$INTERFACE\"\n", subnet_str); break; + case SUBNET_MAC: default: return; } @@ -238,6 +241,7 @@ void ifconfig_route(FILE *out, const char *value) { fprintf(out, "netsh interface ipv6 add route %s \"%%INTERFACE%%\" %s\n", subnet_str, gateway_str); break; + case SUBNET_MAC: default: return; } @@ -251,6 +255,7 @@ void ifconfig_route(FILE *out, const char *value) { fprintf(out, "netsh interface ipv6 add route %s \"%%INTERFACE%%\"\n", subnet_str); break; + case SUBNET_MAC: default: return; } @@ -278,6 +283,7 @@ void ifconfig_route(FILE *out, const char *value) { net2str(gateway_str, sizeof(gateway_str), &ipv6); break; + case SUBNET_MAC: default: return; } @@ -298,6 +304,7 @@ void ifconfig_route(FILE *out, const char *value) { fprintf(out, "route add -inet6 %s %s\n", subnet_str, gateway_str); break; + case SUBNET_MAC: default: return; } diff --git a/src/logger.c b/src/logger.c index 3e630f23..8650a1cc 100644 --- a/src/logger.c +++ b/src/logger.c @@ -239,6 +239,7 @@ void logger(debug_t level, int priority, const char *format, ...) { real_logger(level, priority, message); } +static void sptps_logger(sptps_t *s, int s_errno, const char *format, va_list ap) ATTR_FORMAT(printf, 3, 0); static void sptps_logger(sptps_t *s, int s_errno, const char *format, va_list ap) { (void)s_errno; char message[1024]; diff --git a/src/net_packet.c b/src/net_packet.c index 28459ce5..6fea959f 100644 --- a/src/net_packet.c +++ b/src/net_packet.c @@ -1643,6 +1643,7 @@ void broadcast_packet(const node_t *from, vpn_packet_t *packet) { break; + case BMODE_NONE: default: break; } diff --git a/src/protocol_auth.c b/src/protocol_auth.c index d7cbbd48..7a3e6dd0 100644 --- a/src/protocol_auth.c +++ b/src/protocol_auth.c @@ -82,6 +82,7 @@ static bool send_proxyrequest(connection_t *c) { case PROXY_EXEC: return true; + case PROXY_NONE: default: logger(DEBUG_ALWAYS, LOG_ERR, "Unknown proxy type"); return false; diff --git a/src/script.h b/src/script.h index 381f9523..4d10d2e8 100644 --- a/src/script.h +++ b/src/script.h @@ -29,8 +29,8 @@ typedef struct environment { char **entries; } environment_t; -extern int environment_add(environment_t *env, const char *format, ...); -extern void environment_update(environment_t *env, int pos, const char *format, ...); +extern int environment_add(environment_t *env, const char *format, ...) ATTR_FORMAT(printf, 2, 3); +extern void environment_update(environment_t *env, int pos, const char *format, ...) ATTR_FORMAT(printf, 3, 4); extern void environment_init(environment_t *env); extern void environment_exit(environment_t *env); diff --git a/src/sptps.c b/src/sptps.c index 42eaba0c..35c68bc9 100644 --- a/src/sptps.c +++ b/src/sptps.c @@ -68,6 +68,7 @@ void sptps_log_stderr(sptps_t *s, int s_errno, const char *format, va_list ap) { void (*sptps_log)(sptps_t *s, int s_errno, const char *format, va_list ap) = sptps_log_stderr; // Log an error message. +static bool error(sptps_t *s, int s_errno, const char *format, ...) ATTR_FORMAT(printf, 3, 4); static bool error(sptps_t *s, int s_errno, const char *format, ...) { (void)s; (void)s_errno; @@ -83,6 +84,7 @@ static bool error(sptps_t *s, int s_errno, const char *format, ...) { return false; } +static void warning(sptps_t *s, const char *format, ...) ATTR_FORMAT(printf, 2, 3); static void warning(sptps_t *s, const char *format, ...) { va_list ap; va_start(ap, format); @@ -640,7 +642,7 @@ size_t sptps_receive_data(sptps_t *s, const void *vdata, size_t len) { s->inbuf = realloc(s->inbuf, s->reclen + 19UL); if(!s->inbuf) { - return error(s, errno, strerror(errno)); + return error(s, errno, "%s", strerror(errno)); } // Exit early if we have no more data to process. @@ -718,7 +720,7 @@ bool sptps_start(sptps_t *s, void *handle, bool initiator, bool datagram, ecdsa_ s->late = malloc(s->replaywin); if(!s->late) { - return error(s, errno, strerror(errno)); + return error(s, errno, "%s", strerror(errno)); } memset(s->late, 0, s->replaywin); @@ -727,14 +729,14 @@ bool sptps_start(sptps_t *s, void *handle, bool initiator, bool datagram, ecdsa_ s->label = malloc(labellen); if(!s->label) { - return error(s, errno, strerror(errno)); + return error(s, errno, "%s", strerror(errno)); } if(!datagram) { s->inbuf = malloc(7); if(!s->inbuf) { - return error(s, errno, strerror(errno)); + return error(s, errno, "%s", strerror(errno)); } s->buflen = 0; diff --git a/src/sptps.h b/src/sptps.h index 95209e5d..633eb810 100644 --- a/src/sptps.h +++ b/src/sptps.h @@ -104,9 +104,9 @@ typedef struct sptps { } sptps_t; extern unsigned int sptps_replaywin; -extern void sptps_log_quiet(sptps_t *s, int s_errno, const char *format, va_list ap); -extern void sptps_log_stderr(sptps_t *s, int s_errno, const char *format, va_list ap); -extern void (*sptps_log)(sptps_t *s, int s_errno, const char *format, va_list ap); +extern void sptps_log_quiet(sptps_t *s, int s_errno, const char *format, va_list ap) ATTR_FORMAT(printf, 3, 0); +extern void sptps_log_stderr(sptps_t *s, int s_errno, const char *format, va_list ap) ATTR_FORMAT(printf, 3, 0); +extern void (*sptps_log)(sptps_t *s, int s_errno, const char *format, va_list ap) ATTR_FORMAT(printf, 3, 0); extern bool sptps_start(sptps_t *s, void *handle, bool initiator, bool datagram, ecdsa_t *mykey, ecdsa_t *hiskey, const void *label, size_t labellen, send_data_t send_data, receive_record_t receive_record); extern bool sptps_stop(sptps_t *s); extern bool sptps_send_record(sptps_t *s, uint8_t type, const void *data, uint16_t len); diff --git a/src/sptps_test.c b/src/sptps_test.c index b26615e1..38e8b50d 100644 --- a/src/sptps_test.c +++ b/src/sptps_test.c @@ -154,7 +154,7 @@ static struct option const long_options[] = { }; static void usage(void) { - static const char *message = + fprintf(stderr, "Usage: %s [options] my_ed25519_key_file his_ed25519_key_file [host] port\n" "\n" "Valid options are:\n" @@ -172,9 +172,8 @@ static void usage(void) { " -4 Use IPv4.\n" " -6 Use IPv6.\n" "\n" - "Report bugs to tinc@tinc-vpn.org.\n"; - - fprintf(stderr, message, program_name); + "Report bugs to tinc@tinc-vpn.org.\n", + program_name); } #ifdef HAVE_WINDOWS diff --git a/src/tincctl.c b/src/tincctl.c index b2d98f62..afe19cd2 100644 --- a/src/tincctl.c +++ b/src/tincctl.c @@ -108,7 +108,7 @@ static struct option const long_options[] = { }; static void version(void) { - static const char *message = + fprintf(stdout, "%s version %s (built %s %s, protocol %d.%d)\n" "Features:" #ifdef HAVE_READLINE @@ -126,16 +126,15 @@ static void version(void) { "\n" "tinc comes with ABSOLUTELY NO WARRANTY. This is free software,\n" "and you are welcome to redistribute it under certain conditions;\n" - "see the file COPYING for details.\n"; - - printf(message, PACKAGE, BUILD_VERSION, BUILD_DATE, BUILD_TIME, PROT_MAJOR, PROT_MINOR); + "see the file COPYING for details.\n", + PACKAGE, BUILD_VERSION, BUILD_DATE, BUILD_TIME, PROT_MAJOR, PROT_MINOR); } static void usage(bool status) { if(status) { fprintf(stderr, "Try `%s --help\' for more information.\n", program_name); } else { - static const char *message = + fprintf(stdout, "Usage: %s [options] command\n" "\n" "Valid options are:\n" @@ -194,9 +193,8 @@ static void usage(bool status) { " sign [FILE] Generate a signed version of a file.\n" " verify NODE [FILE] Verify that a file was signed by the given NODE.\n" "\n" - "Report bugs to tinc@tinc-vpn.org.\n"; - - printf(message, program_name); + "Report bugs to tinc@tinc-vpn.org.\n", + program_name); } } diff --git a/src/tincctl.h b/src/tincctl.h index 9bacc46a..94df21d8 100644 --- a/src/tincctl.h +++ b/src/tincctl.h @@ -49,7 +49,7 @@ extern const var_t variables[]; extern size_t rstrip(char *value); extern char *get_my_name(bool verbose) ATTR_MALLOC; extern bool connect_tincd(bool verbose); -extern bool sendline(int fd, const char *format, ...); +extern bool sendline(int fd, const char *format, ...) ATTR_FORMAT(printf, 2, 3); extern bool recvline(int fd, char *line, size_t len); extern int check_port(const char *name); diff --git a/src/tincd.c b/src/tincd.c index 06e67130..edb03f51 100644 --- a/src/tincd.c +++ b/src/tincd.c @@ -130,7 +130,7 @@ static void usage(bool status) { fprintf(stderr, "Try `%s --help\' for more information.\n", program_name); else { - static const char *message = + fprintf(stdout, "Usage: %s [option]...\n" "\n" " -c, --config=DIR Read configuration options from DIR.\n" @@ -152,9 +152,8 @@ static void usage(bool status) { " --help Display this help and exit.\n" " --version Output version information and exit.\n" "\n" - "Report bugs to tinc@tinc-vpn.org.\n"; - - printf(message, program_name); + "Report bugs to tinc@tinc-vpn.org.\n", + program_name); } } @@ -419,7 +418,7 @@ int main(int argc, char **argv) { } if(show_version) { - static const char *message = + fprintf(stdout, "%s version %s (built %s %s, protocol %d.%d)\n" "Features:" #ifdef HAVE_OPENSSL @@ -461,9 +460,8 @@ int main(int argc, char **argv) { "\n" "tinc comes with ABSOLUTELY NO WARRANTY. This is free software,\n" "and you are welcome to redistribute it under certain conditions;\n" - "see the file COPYING for details.\n"; - - printf(message, PACKAGE, BUILD_VERSION, BUILD_DATE, BUILD_TIME, PROT_MAJOR, PROT_MINOR); + "see the file COPYING for details.\n", + PACKAGE, BUILD_VERSION, BUILD_DATE, BUILD_TIME, PROT_MAJOR, PROT_MINOR); return 0; } diff --git a/src/xalloc.h b/src/xalloc.h index c8cfbebf..2b217e8b 100644 --- a/src/xalloc.h +++ b/src/xalloc.h @@ -68,6 +68,7 @@ static inline char *xstrdup(const char *s) { return p; } +static inline int xvasprintf(char **strp, const char *fmt, va_list ap) ATTR_FORMAT(printf, 2, 0); static inline int xvasprintf(char **strp, const char *fmt, va_list ap) { #ifdef HAVE_WINDOWS char buf[1024]; -- 2.20.1