On Tue, Jan 31, 2017 at 09:32:19AM +0000, Matthias Klose wrote:
> cfparse.y:546:39: note: using the range [1, -9223372036854775808] for 
> directive argument
> In file included from /usr/include/stdio.h:938:0,
>                  from cfparse.y:50:
> /usr/include/x86_64-linux-gnu/bits/stdio2.h:64:10: note: format output 
> between 2 and 21 bytes into a destination of size 10
>    return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>         __bos (__s), __fmt, __va_arg_pack ());
>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cfparse.y:1388:39: error: '%lu' directive output may be truncated writing 
> between 1 and 20 bytes into a region of size 10 [-Werror=format-truncation=]
>     snprintf(portbuf, sizeof(portbuf), "%lu", $4);
>                                        ^~~~~
> cfparse.y:1388:39: note: using the range [1, 18446744073709551615] for 
> directive argument

There are several errors during compilation. Most of them follow a
similar pattern to what's above. They generally come down to a bit of
"clever" optimization to save a few bytes here and there. For example,
if we're storing a network prefix length in an int, we know the maximum
value will never need more than 4 bytes when represented as a
null-terminated string, e.g. "128\0". However, an arbitrary int can
require 11 bytes in the same representation. In these cases, we should
simply make the buffers big enough to keep gcc quiet.

Most of these format string issues relate to internal data and interally
allocated buffers, and they don't seem to be particurly problematic.
However, the format string issue when writing entries to utmp definitely
doesn't seem right. Utmpx defined the ut_id field as a 4 byte buffer. In
racoon, we're writing to that buffer with the following:

snprintf(ut.ut_id, sizeof ut.ut_id, TERMSPEC, port);

Where TERMSPEC is defined as "vpn%d". There's really no way that this
can work. I think ut_line is a more appropriate field in utmp for this
information.

Attached is a patch that I believe addresses the issues. I'd appreciate
feedback before I push it.

noah

Index: pkg-ipsec-tools/src/racoon/cfparse.y
===================================================================
--- pkg-ipsec-tools.orig/src/racoon/cfparse.y
+++ pkg-ipsec-tools/src/racoon/cfparse.y
@@ -2564,7 +2564,7 @@ set_isakmp_proposal(rmconf)
 		plog(LLV_DEBUG2, LOCATION, NULL,
 			"encklen=%d\n", s->encklen);
 
-		memset(types, 0, ARRAYLEN(types));
+		memset(types, 0, ARRAYLEN(types) * sizeof(types[0]));
 		types[algclass_isakmp_enc] = s->algclass[algclass_isakmp_enc];
 		types[algclass_isakmp_hash] = s->algclass[algclass_isakmp_hash];
 		types[algclass_isakmp_dh] = s->algclass[algclass_isakmp_dh];
Index: pkg-ipsec-tools/src/racoon/isakmp_cfg.c
===================================================================
--- pkg-ipsec-tools.orig/src/racoon/isakmp_cfg.c
+++ pkg-ipsec-tools/src/racoon/isakmp_cfg.c
@@ -1701,7 +1701,7 @@ isakmp_cfg_accounting_system(port, raddr
 
 	memset(&ut, 0, sizeof ut);
 	gettimeofday((struct timeval *)&ut.ut_tv, NULL);
-	snprintf(ut.ut_id, sizeof ut.ut_id, TERMSPEC, port);
+	snprintf(ut.ut_line, sizeof ut.ut_line, TERMSPEC, port);
 
 	switch (inout) {
 	case ISAKMP_CFG_LOGIN:
@@ -1713,7 +1713,7 @@ isakmp_cfg_accounting_system(port, raddr
 
 		plog(LLV_INFO, LOCATION, NULL,
 			"Accounting : '%s' logging on '%s' from %s.\n",
-			ut.ut_user, ut.ut_id, addr);
+			ut.ut_user, ut.ut_line, addr);
 
 		pututxline(&ut);
 
@@ -1723,7 +1723,7 @@ isakmp_cfg_accounting_system(port, raddr
 
 		plog(LLV_INFO, LOCATION, NULL,
 			"Accounting : '%s' unlogging from '%s'.\n",
-			usr, ut.ut_id);
+			usr, ut.ut_line);
 
 		pututxline(&ut);
 
@@ -1920,7 +1920,7 @@ isakmp_cfg_setenv(iph1, envp, envc)
 	char *splitlist_cidr;
 	char defdom[MAXPATHLEN + 1];
 	int cidr, tmp;
-	char cidrstr[4];
+	char cidrstr[12];
 	int i, p;
 	int test;
 
@@ -1983,7 +1983,7 @@ isakmp_cfg_setenv(iph1, envp, envc)
 	tmp = ntohl(iph1->mode_cfg->mask4.s_addr);
 	for (cidr = 0; tmp != 0; cidr++)
 		tmp <<= 1;
-	snprintf(cidrstr, 3, "%d", cidr);
+	snprintf(cidrstr, 12, "%d", cidr);
 
 	if (script_env_append(envp, envc, "INTERNAL_CIDR4", cidrstr) != 0) {
 		plog(LLV_ERROR, LOCATION, NULL, "Cannot set INTERNAL_CIDR4\n");

Attachment: signature.asc
Description: PGP signature

Reply via email to