From 393e42ae5f8080f8637de505a86c987773d29a12 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Mon, 9 May 2022 10:31:28 +0200 Subject: [PATCH] BUILD: ssl: work around bogus warning in gcc 12's -Wformat-truncation As was first reported by Ilya in issue #1513, Gcc 12 incorrectly reports a possible overflow from the concatenation of two strings whose size was previously checked to fit: src/ssl_crtlist.c: In function 'crtlist_parse_file': src/ssl_crtlist.c:545:58: error: '%s' directive output may be truncated writing up to 4095 bytes into a region of size between 1 and 4096 [-Werror=format-truncation=] 545 | snprintf(path, sizeof(path), "%s/%s", global_ssl.crt_base, crt_path); | ^~ src/ssl_crtlist.c:545:25: note: 'snprintf' output between 2 and 8192 bytes into a destination of size 4097 545 | snprintf(path, sizeof(path), "%s/%s", global_ssl.crt_base, crt_path); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ It would be a bit concerning to disable -Wformat-truncation because it might detect real programming mistakes at other places. The solution adopted in this patch is absolutely ugly and error-prone, but it works, it consists in integrating the snprintf() call in the error condition and to test the result again. Let's hope a smarter compiler will not warn that this test is absurd since guaranteed by the first condition... This may have to be backported for those suffering from a compiler upgrade. --- src/cfgparse-ssl.c | 4 ++-- src/ssl_crtlist.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c index 7acd135ddc..462743e45e 100644 --- a/src/cfgparse-ssl.c +++ b/src/cfgparse-ssl.c @@ -659,11 +659,11 @@ static int bind_parse_crt(char **args, int cur_arg, struct proxy *px, struct bin } if ((*args[cur_arg + 1] != '/' ) && global_ssl.crt_base) { - if ((strlen(global_ssl.crt_base) + 1 + strlen(args[cur_arg + 1]) + 1) > MAXPATHLEN) { + if ((strlen(global_ssl.crt_base) + 1 + strlen(args[cur_arg + 1]) + 1) > sizeof(path) || + snprintf(path, sizeof(path), "%s/%s", global_ssl.crt_base, args[cur_arg + 1]) > sizeof(path)) { memprintf(err, "'%s' : path too long", args[cur_arg]); return ERR_ALERT | ERR_FATAL; } - snprintf(path, sizeof(path), "%s/%s", global_ssl.crt_base, args[cur_arg + 1]); return ssl_sock_load_cert(path, conf, err); } diff --git a/src/ssl_crtlist.c b/src/ssl_crtlist.c index 1615ac51db..f43982f4db 100644 --- a/src/ssl_crtlist.c +++ b/src/ssl_crtlist.c @@ -536,13 +536,13 @@ int crtlist_parse_file(char *file, struct bind_conf *bind_conf, struct proxy *cu } if (*crt_path != '/' && global_ssl.crt_base) { - if ((strlen(global_ssl.crt_base) + 1 + strlen(crt_path)) > MAXPATHLEN) { + if ((strlen(global_ssl.crt_base) + 1 + strlen(crt_path)) > sizeof(path) || + snprintf(path, sizeof(path), "%s/%s", global_ssl.crt_base, crt_path)) { memprintf(err, "parsing [%s:%d]: '%s' : path too long", file, linenum, crt_path); cfgerr |= ERR_ALERT | ERR_FATAL; goto error; } - snprintf(path, sizeof(path), "%s/%s", global_ssl.crt_base, crt_path); crt_path = path; } @@ -1270,12 +1270,12 @@ static int cli_parse_add_crtlist(char **args, char *payload, struct appctx *appc } if (*cert_path != '/' && global_ssl.crt_base) { - if ((strlen(global_ssl.crt_base) + 1 + strlen(cert_path)) > MAXPATHLEN) { + if ((strlen(global_ssl.crt_base) + 1 + strlen(cert_path)) > sizeof(path) || + snprintf(path, sizeof(path), "%s/%s", global_ssl.crt_base, cert_path) > sizeof(path)) { memprintf(&err, "'%s' : path too long", cert_path); cfgerr |= ERR_ALERT | ERR_FATAL; goto error; } - snprintf(path, sizeof(path), "%s/%s", global_ssl.crt_base, cert_path); cert_path = path; } -- 2.39.5