]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUILD: ssl: work around bogus warning in gcc 12's -Wformat-truncation
authorWilly Tarreau <w@1wt.eu>
Mon, 9 May 2022 08:31:28 +0000 (10:31 +0200)
committerWilly Tarreau <w@1wt.eu>
Mon, 9 May 2022 18:32:11 +0000 (20:32 +0200)
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
src/ssl_crtlist.c

index 7acd135ddc8abe427b04ff7f180bb84743e5eaef..462743e45e7329ae67f8ae3c38dfefedb95b382f 100644 (file)
@@ -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);
        }
 
index 1615ac51dbd72eb08f97f7d911c3014c70baf646..f43982f4db598a2ac144a02ee18e1797375c93f8 100644 (file)
@@ -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;
        }