From c179d9a57b41f353b58555e70efa310581bced60 Mon Sep 17 00:00:00 2001 From: Michael Santos Date: Sun, 17 Jun 2018 10:01:44 -0400 Subject: [PATCH 1/6] start_listen_sockets: exit if no addresses Do not allocate a 0 byte buffer if no addresses are available: common.c:122:14: warning: Call to 'malloc' has an allocation size of 0 bytes *sockfd = malloc(num_addr * sizeof(*sockfd[0])); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ --- common.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/common.c b/common.c index ca17354..378eb9a 100644 --- a/common.c +++ b/common.c @@ -116,6 +116,11 @@ int start_listen_sockets(int *sockfd[], struct addrinfo *addr_list) for (addr = addr_list; addr; addr = addr->ai_next) num_addr++; + if (num_addr == 0) { + fprintf(stderr, "FATAL: No available addresses.\n"); + exit(1); + } + if (verbose) fprintf(stderr, "listening to %d addresses\n", num_addr); From 5cf591a25416bd21085bd555f81e7200fad52450 Mon Sep 17 00:00:00 2001 From: Michael Santos Date: Sun, 17 Jun 2018 10:01:44 -0400 Subject: [PATCH 2/6] Avoid segfault with malformed IPv6 address A literal IPv6 address without a trailing bracket will result in a write past the end of the address buffer: ~~~ segfault.conf protocols: ( { name: "tls"; host: "["; port: "8443"; } ); ~~~ ~~~ $ sslh-select -p 127.0.0.1:443 --foreground -F./segfault.conf [: no closing bracket in IPv6 address? Segmentation fault (core dumped) ~~~ --- common.c | 1 + 1 file changed, 1 insertion(+) diff --git a/common.c b/common.c index 378eb9a..997507c 100644 --- a/common.c +++ b/common.c @@ -496,6 +496,7 @@ int resolve_split_name(struct addrinfo **out, const char* ct_host, const char* s end = strrchr(host, ']'); if (!end) { fprintf(stderr, "%s: no closing bracket in IPv6 address?\n", host); + return -1; } host++; /* skip first bracket */ *end = 0; /* remove last bracket */ From cfd0163a5b74e6c198d0bcb4a742492111e8aeae Mon Sep 17 00:00:00 2001 From: Michael Santos Date: Sun, 17 Jun 2018 10:01:44 -0400 Subject: [PATCH 3/6] main_loop: initialize in_socket in_socket may be used uninitialized if no addresses are available. ~~~ sslh-select.c:415:8: warning: Function call argument is an uninitialized value check_access_rights(in_socket, cnx[i].proto->service)) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~ --- sslh-select.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sslh-select.c b/sslh-select.c index 2fa83fe..07265e3 100644 --- a/sslh-select.c +++ b/sslh-select.c @@ -301,7 +301,8 @@ void main_loop(int listen_sockets[], int num_addr_listen) fd_set fds_r, fds_w; /* reference fd sets (used to init the next 2) */ fd_set readfds, writefds; /* working read and write fd sets */ struct timeval tv; - int max_fd, in_socket, i, j, res; + int max_fd, i, j, res; + int in_socket = 0; struct connection *cnx; int num_cnx; /* Number of connections in *cnx */ int num_probing = 0; /* Number of connections currently probing From 4c132e3c8d4858ecbd529d69bfc4d8137dedd851 Mon Sep 17 00:00:00 2001 From: Michael Santos Date: Sun, 17 Jun 2018 10:01:44 -0400 Subject: [PATCH 4/6] config: segfault parsing invalid sni/alpn Check return value of config_setting_get_string_elem() for error before passing the result to strlen(): ~~~ segfault.conf protocols: ( { name: "tls"; host: "localhost"; port: "8443"; sni_hostnames: [ 0 ]; } ); ~~~ --- sslh-main.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/sslh-main.c b/sslh-main.c index b2cabed..f0ec9dd 100644 --- a/sslh-main.c +++ b/sslh-main.c @@ -235,7 +235,7 @@ static void setup_regex_probe(struct proto *p, config_setting_t* probes) static void setup_sni_alpn_list(struct proto *p, config_setting_t* config_items, const char* name, int alpn) { int num_probes, i, max_server_name_len, server_name_len; - const char * config_item; + const char * config_item, *server_name; char** sni_hostname_list; num_probes = config_setting_length(config_items); @@ -246,7 +246,12 @@ static void setup_sni_alpn_list(struct proto *p, config_setting_t* config_items, max_server_name_len = 0; for (i = 0; i < num_probes; i++) { - server_name_len = strlen(config_setting_get_string_elem(config_items, i)); + server_name = config_setting_get_string_elem(config_items, i); + if (server_name == NULL) { + fprintf(stderr, "%s: invalid %s specified\n", p->description, name); + exit(1); + } + server_name_len = strlen(server_name); if(server_name_len > max_server_name_len) max_server_name_len = server_name_len; } From 8ce2b2ea052296f4c86e2a9216f8e43c325cc95f Mon Sep 17 00:00:00 2001 From: Michael Santos Date: Mon, 18 Jun 2018 10:06:23 -0400 Subject: [PATCH 5/6] Check memory allocations succeed --- common.c | 7 +++---- common.h | 7 +++++++ sslh-fork.c | 1 + sslh-main.c | 9 +++++++++ sslh-select.c | 1 + systemd-sslh-generator.c | 6 +++++- tls.c | 6 +++--- 7 files changed, 29 insertions(+), 8 deletions(-) diff --git a/common.c b/common.c index 997507c..a7a47ee 100644 --- a/common.c +++ b/common.c @@ -84,6 +84,7 @@ int get_fd_sockets(int *sockfd[]) if (sd > 0) { int i; *sockfd = malloc(sd * sizeof(*sockfd[0])); + CHECK_ALLOC(*sockfd, "malloc"); for (i = 0; i < sd; i++) { (*sockfd)[i] = SD_LISTEN_FDS_START + i; } @@ -125,6 +126,7 @@ int start_listen_sockets(int *sockfd[], struct addrinfo *addr_list) fprintf(stderr, "listening to %d addresses\n", num_addr); *sockfd = malloc(num_addr * sizeof(*sockfd[0])); + CHECK_ALLOC(*sockfd, "malloc"); for (i = 0, addr = addr_list; i < num_addr && addr; i++, addr = addr->ai_next) { if (!addr) { @@ -314,10 +316,7 @@ int defer_write(struct queue *q, void* data, int data_size) fprintf(stderr, "**** writing deferred on fd %d\n", q->fd); p = realloc(q->begin_deferred_data, data_offset + q->deferred_data_size + data_size); - if (!p) { - perror("realloc"); - exit(1); - } + CHECK_ALLOC(p, "realloc"); q->begin_deferred_data = p; q->deferred_data = p + data_offset; diff --git a/common.h b/common.h index 9da9c6b..e8fad3e 100644 --- a/common.h +++ b/common.h @@ -48,6 +48,13 @@ return res; \ } +#define CHECK_ALLOC(a, str) \ + if (!a) { \ + fprintf(stderr, "%s:%d:", __FILE__, __LINE__); \ + perror(str); \ + exit(1); \ + } + #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0])) #if 1 diff --git a/sslh-fork.c b/sslh-fork.c index 2c0b595..78bf460 100644 --- a/sslh-fork.c +++ b/sslh-fork.c @@ -145,6 +145,7 @@ void main_loop(int listen_sockets[], int num_addr_listen) listener_pid_number = num_addr_listen; listener_pid = malloc(listener_pid_number * sizeof(listener_pid[0])); + CHECK_ALLOC(listener_pid, "malloc"); /* Start one process for each listening address */ for (i = 0; i < num_addr_listen; i++) { diff --git a/sslh-main.c b/sslh-main.c index f0ec9dd..2dbbbd6 100644 --- a/sslh-main.c +++ b/sslh-main.c @@ -210,14 +210,17 @@ static void setup_regex_probe(struct proto *p, config_setting_t* probes) p->probe = get_probe("regex"); probe_list = calloc(num_probes + 1, sizeof(*probe_list)); + CHECK_ALLOC(probe_list, "calloc"); p->data = (void*)probe_list; for (i = 0; i < num_probes; i++) { probe_list[i] = malloc(sizeof(*(probe_list[i]))); + CHECK_ALLOC(probe_list[i], "malloc"); expr = config_setting_get_string_elem(probes, i); res = regcomp(probe_list[i], expr, REG_EXTENDED); if (res) { err = malloc(errsize = regerror(res, probe_list[i], NULL, 0)); + CHECK_ALLOC(err, "malloc"); regerror(res, probe_list[i], err, errsize); fprintf(stderr, "%s:%s\n", expr, err); free(err); @@ -257,10 +260,12 @@ static void setup_sni_alpn_list(struct proto *p, config_setting_t* config_items, } sni_hostname_list = calloc(num_probes + 1, ++max_server_name_len); + CHECK_ALLOC(sni_hostname_list, "calloc"); for (i = 0; i < num_probes; i++) { config_item = config_setting_get_string_elem(config_items, i); sni_hostname_list[i] = malloc(max_server_name_len); + CHECK_ALLOC(sni_hostname_list[i], "malloc"); strcpy (sni_hostname_list[i], config_item); if(verbose) fprintf(stderr, "%s: %s[%d]: %s\n", p->description, name, i, sni_hostname_list[i]); } @@ -303,6 +308,7 @@ static int config_protocols(config_t *config, struct proto **prots) num_prots = config_setting_length(setting); for (i = 0; i < num_prots; i++) { p = calloc(1, sizeof(*p)); + CHECK_ALLOC(p, "calloc"); if (i == 0) *prots = p; if (prev) prev->next = p; prev = p; @@ -443,6 +449,7 @@ static void make_alloptions(void) /* Create all_options, composed of const_options followed by one option per * known protocol */ all_options = calloc(ARRAY_SIZE(const_options) + get_num_builtins(), sizeof(struct option)); + CHECK_ALLOC(all_options, "calloc"); memcpy(all_options, const_options, sizeof(const_options)); append_protocols(all_options, ARRAY_SIZE(const_options) - 1, builtins, get_num_builtins()); } @@ -521,8 +528,10 @@ next_arg: if (!prots) { /* No protocols yet -- create the list */ p = prots = calloc(1, sizeof(*p)); + CHECK_ALLOC(p, "calloc"); } else { p->next = calloc(1, sizeof(*p)); + CHECK_ALLOC(p->next, "calloc"); p = p->next; } memcpy(p, &builtins[c-PROT_SHIFT], sizeof(*p)); diff --git a/sslh-select.c b/sslh-select.c index 07265e3..ebc2cc8 100644 --- a/sslh-select.c +++ b/sslh-select.c @@ -322,6 +322,7 @@ void main_loop(int listen_sockets[], int num_addr_listen) num_cnx = cnx_num_alloc; /* Start with a set pool of slots */ cnx = malloc(num_cnx * sizeof(struct connection)); + CHECK_ALLOC(cnx, "malloc"); for (i = 0; i < num_cnx; i++) init_cnx(&cnx[i]); diff --git a/systemd-sslh-generator.c b/systemd-sslh-generator.c index 9f57c05..01aef63 100644 --- a/systemd-sslh-generator.c +++ b/systemd-sslh-generator.c @@ -7,7 +7,8 @@ static char* resolve_listen(const char *hostname, const char *port) { /* Need room in the strcat for \0 and : * the format in the socket unit file is hostname:port */ - char *conn = (char*)malloc(strlen(hostname)+strlen(port)+2); + char *conn = malloc(strlen(hostname)+strlen(port)+2); + CHECK_ALLOC(conn, "malloc"); strcpy(conn, hostname); strcat(conn, ":"); strcat(conn, port); @@ -41,6 +42,7 @@ static int get_listen_from_conf(const char *filename, char **listen[]) { int i; len = config_setting_length(setting); *listen = malloc(len * sizeof(**listen)); + CHECK_ALLOC(*listen, "malloc"); for (i = 0; i < len; i++) { addr = config_setting_get_elem(setting, i); if (! (config_setting_lookup_string(addr, "host", &hostname) && @@ -51,6 +53,7 @@ static int get_listen_from_conf(const char *filename, char **listen[]) { return -1; } else { (*listen)[i] = malloc(strlen(resolve_listen(hostname, port))); + CHECK_ALLOC((*listen)[i], "malloc"); strcpy((*listen)[i], resolve_listen(hostname, port)); } } @@ -115,6 +118,7 @@ static int gen_sslh_config(char *runtime_unit_dir) { size_t uf_len = strlen(unit_file); size_t runtime_len = strlen(runtime_unit_dir) + uf_len + 1; char *runtime_conf = malloc(runtime_len); + CHECK_ALLOC(runtime_conf, "malloc"); strcpy(runtime_conf, runtime_unit_dir); strcat(runtime_conf, unit_file); runtime_conf_fd = fopen(runtime_conf, "w"); diff --git a/tls.c b/tls.c index 4144f72..5a591f4 100644 --- a/tls.c +++ b/tls.c @@ -292,6 +292,7 @@ static int has_match(char** list, const char* name, size_t name_len) { char **item; char *name_nullterminated = malloc(name_len+1); + CHECK_ALLOC(name_nullterminated, "malloc"); memcpy(name_nullterminated, name, name_len); name_nullterminated[name_len]='\0'; @@ -309,9 +310,8 @@ has_match(char** list, const char* name, size_t name_len) { struct TLSProtocol * new_tls_data() { struct TLSProtocol *tls_data = malloc(sizeof(struct TLSProtocol)); - if (tls_data != NULL) { - tls_data->use_alpn = -1; - } + CHECK_ALLOC(tls_data, "malloc"); + tls_data->use_alpn = -1; return tls_data; } From 9228171eb000fd074ad7b556edd70b674fa01504 Mon Sep 17 00:00:00 2001 From: Michael Santos Date: Mon, 18 Jun 2018 10:07:49 -0400 Subject: [PATCH 6/6] config: exit if list element is invalid --- sslh-main.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sslh-main.c b/sslh-main.c index 2dbbbd6..663b9d6 100644 --- a/sslh-main.c +++ b/sslh-main.c @@ -217,6 +217,10 @@ static void setup_regex_probe(struct proto *p, config_setting_t* probes) probe_list[i] = malloc(sizeof(*(probe_list[i]))); CHECK_ALLOC(probe_list[i], "malloc"); expr = config_setting_get_string_elem(probes, i); + if (expr == NULL) { + fprintf(stderr, "%s: invalid probe specified\n", p->description); + exit(1); + } res = regcomp(probe_list[i], expr, REG_EXTENDED); if (res) { err = malloc(errsize = regerror(res, probe_list[i], NULL, 0)); @@ -264,6 +268,10 @@ static void setup_sni_alpn_list(struct proto *p, config_setting_t* config_items, for (i = 0; i < num_probes; i++) { config_item = config_setting_get_string_elem(config_items, i); + if (config_item == NULL) { + fprintf(stderr, "%s: invalid %s specified\n", p->description, name); + exit(1); + } sni_hostname_list[i] = malloc(max_server_name_len); CHECK_ALLOC(sni_hostname_list[i], "malloc"); strcpy (sni_hostname_list[i], config_item);