treewide: replace unsafe string functions

Replace sprintf(), strncpy() etc. with safer variants that perform bounds
checking on the target buffer. Also rework unsafe `p += sprintf(p, ....)`
code to properly handle error cases.

Ref: http://lists.infradead.org/pipermail/openwrt-devel/2020-May/023363.html
Suggested-by: Philip Prindeville <philipp@redfish-solutions.com>
Signed-off-by: Jo-Philipp Wich <jo@mein.io>

defaults.c

@@ -218,7 +218,7 @@ fw3_print_default_head_rules(struct fw3_ipt_handle *handle,
 {
 	int i;
 	struct fw3_defaults *defs = &state->defaults;
-	struct fw3_device lodev = { .set = true };
+	struct fw3_device lodev = { .set = true, .name = "lo" };
 	struct fw3_protocol tcp = { .protocol = 6 };
 	struct fw3_ipt_rule *r;
 
@@ -232,8 +232,6 @@ fw3_print_default_head_rules(struct fw3_ipt_handle *handle,
 	{
 	case FW3_TABLE_FILTER:
 
-		sprintf(lodev.name, "lo");
-
 		r = fw3_ipt_rule_create(handle, NULL, &lodev, NULL, NULL, NULL);
 		fw3_ipt_rule_target(r, "ACCEPT");
 		fw3_ipt_rule_append(r, "INPUT");
@@ -378,7 +376,7 @@ static void
 set_default(const char *name, int set)
 {
 	FILE *f;
-	char path[sizeof("/proc/sys/net/ipv4/tcp_window_scaling\0")];
+	char path[sizeof("/proc/sys/net/ipv4/tcp_window_scaling")];
 
 	snprintf(path, sizeof(path), "/proc/sys/net/ipv4/tcp_%s", name);
 

includes.c

@@ -182,19 +182,14 @@ fw3_print_includes(struct fw3_state *state, enum fw3_family family, bool reload)
 		fw3_command_close();
 }
 
+#define TEMPLATE "config() { echo \"You cannot use UCI in firewall includes!\" >&2; exit 1; }; . %s"
 
 static void
 run_include(struct fw3_include *include)
 {
 	int rv;
 	struct stat s;
-	const char *tmpl =
-		"config() { "
-			"echo \"You cannot use UCI in firewall includes!\" >&2; "
-			"exit 1; "
-		"}; . %s";
-
-	char buf[PATH_MAX + sizeof(tmpl)];
+	char buf[PATH_MAX + sizeof(TEMPLATE)];
 
 	info(" * Running script '%s'", include->path);
 
@@ -204,7 +199,7 @@ run_include(struct fw3_include *include)
 		return;
 	}
 
-	snprintf(buf, sizeof(buf), tmpl, include->path);
+	snprintf(buf, sizeof(buf), TEMPLATE, include->path);
 	rv = system(buf);
 
 	if (rv)

iptables.c

@@ -141,12 +141,11 @@ void
 get_kernel_version(void)
 {
 	static struct utsname uts;
-	int x = 0, y = 0, z = 0;
+	int x = 3, y = 0, z = 0;
 
-	if (uname(&uts) == -1)
-		sprintf(uts.release, "3.0.0");
+	if (uname(&uts) != -1)
+		sscanf(uts.release, "%d.%d.%d", &x, &y, &z);
 
-	sscanf(uts.release, "%d.%d.%d", &x, &y, &z);
 	kernel_version = 0x10000 * x + 0x100 * y + z;
 }
 
@@ -491,22 +490,15 @@ is_chain(struct fw3_ipt_handle *h, const char *name)
 
 void
 fw3_ipt_create_chain(struct fw3_ipt_handle *h, bool ignore_existing,
-                     const char *fmt, ...)
+                     const char *chain)
 {
-	char buf[32];
-	va_list ap;
-
-	va_start(ap, fmt);
-	vsnprintf(buf, sizeof(buf) - 1, fmt, ap);
-	va_end(ap);
-
-	if (ignore_existing && is_chain(h, buf))
+	if (ignore_existing && is_chain(h, chain))
 		return;
 
 	if (fw3_pr_debug)
-		debug(h, "-N %s\n", buf);
+		debug(h, "-N %s\n", chain);
 
-	iptc_create_chain(buf, h->handle);
+	iptc_create_chain(chain, h->handle);
 }
 
 void
@@ -952,7 +944,7 @@ void
 fw3_ipt_rule_sport_dport(struct fw3_ipt_rule *r,
                          struct fw3_port *sp, struct fw3_port *dp)
 {
-	char buf[sizeof("65535:65535\0")];
+	char buf[sizeof("65535:65535")];
 
 	if ((!sp || !sp->set) && (!dp || !dp->set))
 		return;
@@ -963,7 +955,7 @@ fw3_ipt_rule_sport_dport(struct fw3_ipt_rule *r,
 	if (sp && sp->set)
 	{
 		if (sp->port_min == sp->port_max)
-			sprintf(buf, "%u", sp->port_min);
+			snprintf(buf, sizeof(buf), "%u", sp->port_min);
 		else
 			snprintf(buf, sizeof(buf), "%u:%u", sp->port_min, sp->port_max);
 
@@ -973,7 +965,7 @@ fw3_ipt_rule_sport_dport(struct fw3_ipt_rule *r,
 	if (dp && dp->set)
 	{
 		if (dp->port_min == dp->port_max)
-			sprintf(buf, "%u", dp->port_min);
+			snprintf(buf, sizeof(buf), "%u", dp->port_min);
 		else
 			snprintf(buf, sizeof(buf), "%u:%u", dp->port_min, dp->port_max);
 
@@ -984,9 +976,10 @@ fw3_ipt_rule_sport_dport(struct fw3_ipt_rule *r,
 void
 fw3_ipt_rule_device(struct fw3_ipt_rule *r, const char *device, bool out)
 {
+	struct fw3_device dev = { .any = false };
+
 	if (device) {
-		struct fw3_device dev = { .any = false };
-		strncpy(dev.name, device, sizeof(dev.name) - 1);
+		snprintf(dev.name, sizeof(dev.name), "%s", device);
 		fw3_ipt_rule_in_out(r, (out) ? NULL : &dev, (out) ? &dev : NULL);
 	}
 }
@@ -994,14 +987,14 @@ fw3_ipt_rule_device(struct fw3_ipt_rule *r, const char *device, bool out)
 void
 fw3_ipt_rule_mac(struct fw3_ipt_rule *r, struct fw3_mac *mac)
 {
-	char buf[sizeof("ff:ff:ff:ff:ff:ff\0")];
+	char buf[sizeof("ff:ff:ff:ff:ff:ff")];
 	uint8_t *addr = mac->mac.ether_addr_octet;
 
 	if (!mac)
 		return;
 
-	sprintf(buf, "%02x:%02x:%02x:%02x:%02x:%02x",
-	        addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]);
+	snprintf(buf, sizeof(buf), "%02x:%02x:%02x:%02x:%02x:%02x",
+	         addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]);
 
 	fw3_ipt_rule_addarg(r, false, "-m", "mac");
 	fw3_ipt_rule_addarg(r, mac->invert, "--mac-source", buf);
@@ -1010,7 +1003,7 @@ fw3_ipt_rule_mac(struct fw3_ipt_rule *r, struct fw3_mac *mac)
 void
 fw3_ipt_rule_icmptype(struct fw3_ipt_rule *r, struct fw3_icmptype *icmp)
 {
-	char buf[sizeof("255/255\0")];
+	char buf[sizeof("255/255")];
 
 	if (!icmp)
 		return;
@@ -1019,7 +1012,7 @@ fw3_ipt_rule_icmptype(struct fw3_ipt_rule *r, struct fw3_icmptype *icmp)
 	if (r->h->family == FW3_FAMILY_V6)
 	{
 		if (icmp->code6_min == 0 && icmp->code6_max == 0xFF)
-			sprintf(buf, "%u", icmp->type6);
+			snprintf(buf, sizeof(buf), "%u", icmp->type6);
 		else
 			snprintf(buf, sizeof(buf), "%u/%u", icmp->type6, icmp->code6_min);
 
@@ -1040,19 +1033,19 @@ fw3_ipt_rule_icmptype(struct fw3_ipt_rule *r, struct fw3_icmptype *icmp)
 void
 fw3_ipt_rule_limit(struct fw3_ipt_rule *r, struct fw3_limit *limit)
 {
-	char buf[sizeof("-4294967296/second\0")];
+	char buf[sizeof("-4294967296/second")];
 
 	if (!limit || limit->rate <= 0)
 		return;
 
 	fw3_ipt_rule_addarg(r, false, "-m", "limit");
 
-	sprintf(buf, "%u/%s", limit->rate, fw3_limit_units[limit->unit]);
+	snprintf(buf, sizeof(buf), "%u/%s", limit->rate, fw3_limit_units[limit->unit]);
 	fw3_ipt_rule_addarg(r, limit->invert, "--limit", buf);
 
 	if (limit->burst > 0)
 	{
-		sprintf(buf, "%u", limit->burst);
+		snprintf(buf, sizeof(buf), "%u", limit->burst);
 		fw3_ipt_rule_addarg(r, limit->invert, "--limit-burst", buf);
 	}
 }
@@ -1060,9 +1053,10 @@ fw3_ipt_rule_limit(struct fw3_ipt_rule *r, struct fw3_limit *limit)
 void
 fw3_ipt_rule_ipset(struct fw3_ipt_rule *r, struct fw3_setmatch *match)
 {
-	char buf[sizeof("dst,dst,dst\0")];
+	char buf[sizeof("dst,dst,dst")];
 	char *p = buf;
-	int i = 0;
+	int i = 0, len;
+	size_t rem = sizeof(buf);
 
 	struct fw3_ipset *set;
 	struct fw3_ipset_datatype *type;
@@ -1076,10 +1070,23 @@ fw3_ipt_rule_ipset(struct fw3_ipt_rule *r, struct fw3_setmatch *match)
 		if (i >= 3)
 			break;
 
-		if (p > buf)
+		if (p > buf) {
+			if (rem <= 1)
+				break;
+
 			*p++ = ',';
+			*p = 0;
+			rem--;
+		}
+
+		len = snprintf(p, rem, "%s", match->dir[i] ? match->dir[i] : type->dir);
+
+		if (len < 0 || len >= rem)
+			break;
+
+		rem -= len;
+		p += len;
 
-		p += sprintf(p, "%s", match->dir[i] ? match->dir[i] : type->dir);
 		i++;
 	}
 
@@ -1104,15 +1111,17 @@ fw3_ipt_rule_helper(struct fw3_ipt_rule *r, struct fw3_cthelpermatch *match)
 void
 fw3_ipt_rule_time(struct fw3_ipt_rule *r, struct fw3_time *time)
 {
-	int i;
+	int i, len;
 	struct tm empty = { 0 };
 
-	char buf[84]; /* sizeof("1,2,3,...,30,31\0") */
+	char buf[84]; /* sizeof("1,2,3,...,30,31") */
 	char *p;
 
 	bool d1 = memcmp(&time->datestart, &empty, sizeof(empty));
 	bool d2 = memcmp(&time->datestop, &empty, sizeof(empty));
 
+	size_t rem;
+
 	if (!d1 && !d2 && !time->timestart && !time->timestop &&
 	    !(time->monthdays & 0xFFFFFFFE) && !(time->weekdays & 0xFE))
 	{
@@ -1138,7 +1147,7 @@ fw3_ipt_rule_time(struct fw3_ipt_rule *r, struct fw3_time *time)
 
 	if (time->timestart)
 	{
-		sprintf(buf, "%02d:%02d:%02d",
+		snprintf(buf, sizeof(buf), "%02d:%02d:%02d",
 		        time->timestart / 3600,
 		        time->timestart % 3600 / 60,
 		        time->timestart % 60);
@@ -1148,7 +1157,7 @@ fw3_ipt_rule_time(struct fw3_ipt_rule *r, struct fw3_time *time)
 
 	if (time->timestop)
 	{
-		sprintf(buf, "%02d:%02d:%02d",
+		snprintf(buf, sizeof(buf), "%02d:%02d:%02d",
 		        time->timestop / 3600,
 		        time->timestop % 3600 / 60,
 		        time->timestop % 60);
@@ -1158,14 +1167,26 @@ fw3_ipt_rule_time(struct fw3_ipt_rule *r, struct fw3_time *time)
 
 	if (time->monthdays & 0xFFFFFFFE)
 	{
-		for (i = 1, p = buf; i < 32; i++)
+		for (i = 1, p = buf, rem = sizeof(buf); i < 32; i++)
 		{
 			if (fw3_hasbit(time->monthdays, i))
 			{
-				if (p > buf)
+				if (p > buf) {
+					if (rem <= 1)
+						break;
+
 					*p++ = ',';
+					*p = 0;
+					rem--;
+				}
+
+				len = snprintf(p, rem, "%u", i);
+
+				if (len < 0 || len >= rem)
+					break;
 
-				p += sprintf(p, "%u", i);
+				rem -= len;
+				p += len;
 			}
 		}
 
@@ -1174,14 +1195,26 @@ fw3_ipt_rule_time(struct fw3_ipt_rule *r, struct fw3_time *time)
 
 	if (time->weekdays & 0xFE)
 	{
-		for (i = 1, p = buf; i < 8; i++)
+		for (i = 1, p = buf, rem = sizeof(buf); i < 8; i++)
 		{
 			if (fw3_hasbit(time->weekdays, i))
 			{
-				if (p > buf)
+				if (p > buf) {
+					if (rem <= 1)
+						break;
+
 					*p++ = ',';
+					*p = 0;
+					rem--;
+				}
+
+				p += snprintf(p, rem, "%u", i);
 
-				p += sprintf(p, "%u", i);
+				if (len < 0 || len >= rem)
+					break;
+
+				rem -= len;
+				p += len;
 			}
 		}
 
@@ -1192,15 +1225,15 @@ fw3_ipt_rule_time(struct fw3_ipt_rule *r, struct fw3_time *time)
 void
 fw3_ipt_rule_mark(struct fw3_ipt_rule *r, struct fw3_mark *mark)
 {
-	char buf[sizeof("0xFFFFFFFF/0xFFFFFFFF\0")];
+	char buf[sizeof("0xFFFFFFFF/0xFFFFFFFF")];
 
 	if (!mark || !mark->set)
 		return;
 
 	if (mark->mask < 0xFFFFFFFF)
-		sprintf(buf, "0x%x/0x%x", mark->mark, mark->mask);
+		snprintf(buf, sizeof(buf), "0x%x/0x%x", mark->mark, mark->mask);
 	else
-		sprintf(buf, "0x%x", mark->mark);
+		snprintf(buf, sizeof(buf), "0x%x", mark->mark);
 
 	fw3_ipt_rule_addarg(r, false, "-m", "mark");
 	fw3_ipt_rule_addarg(r, mark->invert, "--mark", buf);
@@ -1209,12 +1242,12 @@ fw3_ipt_rule_mark(struct fw3_ipt_rule *r, struct fw3_mark *mark)
 void
 fw3_ipt_rule_dscp(struct fw3_ipt_rule *r, struct fw3_dscp *dscp)
 {
-	char buf[sizeof("0xFF\0")];
+	char buf[sizeof("0xFF")];
 
 	if (!dscp || !dscp->set)
 		return;
 
-	sprintf(buf, "0x%x", dscp->dscp);
+	snprintf(buf, sizeof(buf), "0x%x", dscp->dscp);
 
 	fw3_ipt_rule_addarg(r, false, "-m", "dscp");
 	fw3_ipt_rule_addarg(r, dscp->invert, "--dscp", buf);
@@ -1230,7 +1263,7 @@ fw3_ipt_rule_comment(struct fw3_ipt_rule *r, const char *fmt, ...)
 		return;
 
 	va_start(ap, fmt);
-	vsnprintf(buf, sizeof(buf) - 1, fmt, ap);
+	vsnprintf(buf, sizeof(buf), fmt, ap);
 	va_end(ap);
 
 	fw3_ipt_rule_addarg(r, false, "-m", "comment");
@@ -1323,7 +1356,7 @@ static void
 rule_print4(struct ipt_entry *e)
 {
 	struct in_addr in_zero = { 0 };
-	char buf1[sizeof("255.255.255.255\0")], buf2[sizeof("255.255.255.255\0")];
+	char buf1[sizeof("255.255.255.255")], buf2[sizeof("255.255.255.255")];
 	char *pname;
 
 	if (e->ip.proto)

iptables.h

@@ -56,7 +56,7 @@ void fw3_ipt_delete_chain(struct fw3_ipt_handle *h, bool if_unused,
 void fw3_ipt_delete_id_rules(struct fw3_ipt_handle *h, const char *chain);
 
 void fw3_ipt_create_chain(struct fw3_ipt_handle *h, bool ignore_existing,
-                          const char *fmt, ...);
+                          const char *chain);
 
 void fw3_ipt_flush(struct fw3_ipt_handle *h);
 

options.c

@@ -939,7 +939,7 @@ fw3_parse_setmatch(void *ptr, const char *val, bool is_list)
 		return false;
 	}
 
-	strncpy(m->name, p, sizeof(m->name) - 1);
+	snprintf(m->name, sizeof(m->name), "%s", p);
 
 	for (i = 0, p = strtok(NULL, " \t,");
 	     i < 3 && p != NULL;
@@ -987,7 +987,7 @@ fw3_parse_cthelper(void *ptr, const char *val, bool is_list)
 	if (*val)
 	{
 		m.set = true;
-		strncpy(m.name, val, sizeof(m.name) - 1);
+		snprintf(m.name, sizeof(m.name), "%s", val);
 		put_value(ptr, &m, sizeof(m), is_list);
 		return true;
 	}
@@ -1239,35 +1239,46 @@ fw3_address_to_string(struct fw3_address *address, bool allow_invert, bool as_ci
 {
 	char *p, ip[INET6_ADDRSTRLEN];
 	static char buf[INET6_ADDRSTRLEN * 2 + 2];
+	size_t rem = sizeof(buf);
+	int len;
 
 	p = buf;
 
-	if (address->invert && allow_invert)
-		p += sprintf(p, "!");
+	if (address->invert && allow_invert) {
+		*p++ = '!';
+		*p = 0;
+		rem--;
+	}
 
 	inet_ntop(address->family == FW3_FAMILY_V4 ? AF_INET : AF_INET6,
 	          &address->address.v4, ip, sizeof(ip));
 
-	p += sprintf(p, "%s", ip);
+	len = snprintf(p, rem, "%s", ip);
+
+	if (len < 0 || len >= rem)
+		return buf;
+
+	rem -= len;
+	p += len;
 
 	if (address->range)
 	{
 		inet_ntop(address->family == FW3_FAMILY_V4 ? AF_INET : AF_INET6,
 		          &address->mask.v4, ip, sizeof(ip));
 
-		p += sprintf(p, "-%s", ip);
+		snprintf(p, rem, "-%s", ip);
 	}
 	else if (!as_cidr)
 	{
 		inet_ntop(address->family == FW3_FAMILY_V4 ? AF_INET : AF_INET6,
 		          &address->mask.v4, ip, sizeof(ip));
 
-		p += sprintf(p, "/%s", ip);
+		snprintf(p, rem, "/%s", ip);
 	}
 	else
 	{
-		p += sprintf(p, "/%u", fw3_netmask2bitlen(address->family,
-		                                          &address->mask.v6));
+		snprintf(p, rem, "/%u",
+		         fw3_netmask2bitlen(address->family, &address->mask.v6));
 	}
 
 	return buf;

redirects.c

@@ -155,7 +155,7 @@ resolve_dest(struct uci_element *e, struct fw3_redirect *redir,
 			if (!compare_addr(addr, &redir->ip_redir))
 				continue;
 
-			strncpy(redir->dest.name, zone->name, sizeof(redir->dest.name) - 1);
+			snprintf(redir->dest.name, sizeof(redir->dest.name), "%s", zone->name);
 			redir->dest.set = true;
 			redir->_dest = zone;
 
@@ -458,14 +458,14 @@ append_chain_nat(struct fw3_ipt_rule *r, struct fw3_redirect *redir)
 static void
 set_redirect(struct fw3_ipt_rule *r, struct fw3_port *port)
 {
-	char buf[sizeof("65535-65535\0")];
+	char buf[sizeof("65535-65535")];
 
 	fw3_ipt_rule_target(r, "REDIRECT");
 
 	if (port && port->set)
 	{
 		if (port->port_min == port->port_max)
-			sprintf(buf, "%u", port->port_min);
+			snprintf(buf, sizeof(buf), "%u", port->port_min);
 		else
 			snprintf(buf, sizeof(buf), "%u-%u", port->port_min, port->port_max);
 
@@ -477,22 +477,30 @@ static void
 set_snat_dnat(struct fw3_ipt_rule *r, enum fw3_flag target,
               struct fw3_address *addr, struct fw3_port *port)
 {
-	char buf[sizeof("255.255.255.255:65535-65535\0")];
-
-	buf[0] = '\0';
+	char buf[sizeof("255.255.255.255:65535-65535")] = {};
+	char ip[INET_ADDRSTRLEN], *p = buf;
+	size_t rem = sizeof(buf);
+	int len;
 
 	if (addr && addr->set)
 	{
-		inet_ntop(AF_INET, &addr->address.v4, buf, sizeof(buf));
+		inet_ntop(AF_INET, &addr->address.v4, ip, sizeof(ip));
+
+		len = snprintf(p, rem, "%s", ip);
+
+		if (len < 0 || len >= rem)
+			return;
+
+		rem -= len;
+		p += len;
 	}
 
 	if (port && port->set)
 	{
 		if (port->port_min == port->port_max)
-			sprintf(buf + strlen(buf), ":%u", port->port_min);
+			snprintf(p, rem, ":%u", port->port_min);
 		else
-			sprintf(buf + strlen(buf), ":%u-%u",
-			        port->port_min, port->port_max);
+			snprintf(p, rem, ":%u-%u", port->port_min, port->port_max);
 	}
 
 	if (target == FW3_FLAG_DNAT)

rules.c

@@ -353,21 +353,21 @@ static void set_target(struct fw3_ipt_rule *r, struct fw3_rule *rule)
 {
 	const char *name;
 	struct fw3_mark *mark;
-	char buf[sizeof("0xFFFFFFFF/0xFFFFFFFF\0")];
+	char buf[sizeof("0xFFFFFFFF/0xFFFFFFFF")];
 
 	switch(rule->target)
 	{
 	case FW3_FLAG_MARK:
 		name = rule->set_mark.set ? "--set-mark" : "--set-xmark";
 		mark = rule->set_mark.set ? &rule->set_mark : &rule->set_xmark;
-		sprintf(buf, "0x%x/0x%x", mark->mark, mark->mask);
+		snprintf(buf, sizeof(buf), "0x%x/0x%x", mark->mark, mark->mask);
 
 		fw3_ipt_rule_target(r, "MARK");
 		fw3_ipt_rule_addarg(r, false, name, buf);
 		return;
 
 	case FW3_FLAG_DSCP:
-		sprintf(buf, "0x%x", rule->set_dscp.dscp);
+		snprintf(buf, sizeof(buf), "0x%x", rule->set_dscp.dscp);
 
 		fw3_ipt_rule_target(r, "DSCP");
 		fw3_ipt_rule_addarg(r, false, "--set-dscp", buf);

snats.c

@@ -265,30 +265,38 @@ static void
 set_target(struct fw3_ipt_rule *r, struct fw3_snat *snat,
            struct fw3_protocol *proto)
 {
-	char buf[sizeof("255.255.255.255:65535-65535\0")];
+	char buf[sizeof("255.255.255.255:65535-65535")] = {};
+	char ip[INET_ADDRSTRLEN], portcntbuf[6], *p = buf;
+	size_t rem = sizeof(buf);
+	int len;
 
 	if (snat->target == FW3_FLAG_SNAT)
 	{
-		buf[0] = '\0';
-
 		if (snat->ip_snat.set)
 		{
-			inet_ntop(AF_INET, &snat->ip_snat.address.v4, buf, sizeof(buf));
+			inet_ntop(AF_INET, &snat->ip_snat.address.v4, ip, sizeof(ip));
+
+			len = snprintf(p, rem, "%s", ip);
+
+			if (len < 0 || len >= rem)
+				return;
+
+			rem -= len;
+			p += len;
 		}
 
 		if (snat->port_snat.set && proto && !proto->any &&
 		    (proto->protocol == 6 || proto->protocol == 17 || proto->protocol == 1))
 		{
 			if (snat->port_snat.port_min == snat->port_snat.port_max)
-				sprintf(buf + strlen(buf), ":%u", snat->port_snat.port_min);
+				snprintf(p, rem, ":%u", snat->port_snat.port_min);
 			else
-				sprintf(buf + strlen(buf), ":%u-%u",
-						snat->port_snat.port_min, snat->port_snat.port_max);
+				snprintf(p, rem, ":%u-%u",
+						 snat->port_snat.port_min, snat->port_snat.port_max);
 
 			if (snat->connlimit_ports) {
-				char portcntbuf[6];
 				snprintf(portcntbuf, sizeof(portcntbuf), "%u",
-						1 + snat->port_snat.port_max - snat->port_snat.port_min);
+				         1 + snat->port_snat.port_max - snat->port_snat.port_min);
 
 				fw3_ipt_rule_addarg(r, false, "-m", "connlimit");
 				fw3_ipt_rule_addarg(r, false, "--connlimit-daddr", NULL);

utils.c

@@ -191,8 +191,7 @@ fw3_find_command(const char *cmd)
 		if ((plen + clen) >= sizeof(path))
 			continue;
 
-		strncpy(path, search, plen);
-		sprintf(path + plen, "/%s", cmd);
+		snprintf(path, sizeof(path), "%.*s/%s", plen, search, cmd);
 
 		if (!stat(path, &s) && S_ISREG(s.st_mode))
 			return path;
@@ -429,7 +428,7 @@ static void
 write_defaults_uci(struct uci_context *ctx, struct fw3_defaults *d,
                    struct uci_package *dest)
 {
-	char buf[sizeof("0xffffffff\0")];
+	char buf[sizeof("0xffffffff")];
 	struct uci_ptr ptr = { .p = dest };
 
 	uci_add_section(ctx, dest, "defaults", &ptr.s);
@@ -449,13 +448,13 @@ write_defaults_uci(struct uci_context *ctx, struct fw3_defaults *d,
 	ptr.value  = fw3_flag_names[d->policy_forward];
 	uci_set(ctx, &ptr);
 
-	sprintf(buf, "0x%x", d->flags[0]);
+	snprintf(buf, sizeof(buf), "0x%x", d->flags[0]);
 	ptr.o      = NULL;
 	ptr.option = "__flags_v4";
 	ptr.value  = buf;
 	uci_set(ctx, &ptr);
 
-	sprintf(buf, "0x%x", d->flags[1]);
+	snprintf(buf, sizeof(buf), "0x%x", d->flags[1]);
 	ptr.o      = NULL;
 	ptr.option = "__flags_v6";
 	ptr.value  = buf;
@@ -612,13 +611,13 @@ write_zone_uci(struct uci_context *ctx, struct fw3_zone *z,
 		uci_set(ctx, &ptr);
 	}
 
-	sprintf(buf, "0x%x", z->flags[0]);
+	snprintf(buf, sizeof(buf), "0x%x", z->flags[0]);
 	ptr.o      = NULL;
 	ptr.option = "__flags_v4";
 	ptr.value  = buf;
 	uci_set(ctx, &ptr);
 
-	sprintf(buf, "0x%x", z->flags[1]);
+	snprintf(buf, sizeof(buf), "0x%x", z->flags[1]);
 	ptr.o      = NULL;
 	ptr.option = "__flags_v6";
 	ptr.value  = buf;
@@ -631,7 +630,7 @@ write_ipset_uci(struct uci_context *ctx, struct fw3_ipset *s,
 {
 	struct fw3_ipset_datatype *type;
 
-	char buf[sizeof("65535-65535\0")];
+	char buf[sizeof("65535-65535")];
 
 	struct uci_ptr ptr = { .p = dest };
 
@@ -660,7 +659,7 @@ write_ipset_uci(struct uci_context *ctx, struct fw3_ipset *s,
 
 	list_for_each_entry(type, &s->datatypes, list)
 	{
-		sprintf(buf, "%s_%s", type->dir, fw3_ipset_type_names[type->type]);
+		snprintf(buf, sizeof(buf), "%s_%s", type->dir, fw3_ipset_type_names[type->type]);
 		ptr.o      = NULL;
 		ptr.option = "match";
 		ptr.value  = buf;
@@ -677,7 +676,7 @@ write_ipset_uci(struct uci_context *ctx, struct fw3_ipset *s,
 
 	if (s->portrange.set)
 	{
-		sprintf(buf, "%u-%u", s->portrange.port_min, s->portrange.port_max);
+		snprintf(buf, sizeof(buf), "%u-%u", s->portrange.port_min, s->portrange.port_max);
 		ptr.o      = NULL;
 		ptr.option = "portrange";
 		ptr.value  = buf;
@@ -1021,7 +1020,7 @@ fw3_check_loopback_dev(const char *name)
 		return false;
 
 	memset(&ifr, 0, sizeof(ifr));
-	strncpy(ifr.ifr_name, name, sizeof(ifr.ifr_name) - 1);
+	snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", name);
 
 	if (ioctl(s, SIOCGIFFLAGS, &ifr) >= 0) {
 		if (ifr.ifr_flags & IFF_LOOPBACK)

xtables-10.h

@@ -45,10 +45,8 @@ fw3_xt_get_match_name(struct xtables_match *m)
 static inline void
 fw3_xt_set_match_name(struct xtables_match *m)
 {
-    if (m->real_name)
-        strcpy(m->m->u.user.name, m->real_name);
-    else
-        strcpy(m->m->u.user.name, m->name);
+    snprintf(m->m->u.user.name, sizeof(m->m->u.user.name), "%s",
+             m->real_name ? m->real_name : m->name);
 }
 
 static inline bool
@@ -92,10 +90,8 @@ fw3_xt_get_target_name(struct xtables_target *t)
 static inline void
 fw3_xt_set_target_name(struct xtables_target *t, const char *name)
 {
-    if (t->real_name)
-        strcpy(t->t->u.user.name, t->real_name);
-    else
-        strcpy(t->t->u.user.name, name);
+    snprintf(t->t->u.user.name, sizeof(t->t->u.user.name), "%s",
+             t->real_name ? t->real_name : name);
 }
 
 static inline bool

xtables-5.h

@@ -36,7 +36,7 @@ fw3_xt_get_match_name(struct xtables_match *m)
 static inline void
 fw3_xt_set_match_name(struct xtables_match *m)
 {
-    strcpy(m->m->u.user.name, m->name);
+    snprintf(m->m->u.user.name, sizeof(m->m->u.user.name), "%s", m->name);
 }
 
 static inline bool
@@ -67,7 +67,7 @@ fw3_xt_get_target_name(struct xtables_target *t)
 static inline void
 fw3_xt_set_target_name(struct xtables_target *t, const char *name)
 {
-    strcpy(t->t->u.user.name, name);
+    snprintf(t->t->u.user.name, sizeof(t->t->u.user.name), "%s", name);
 }
 
 static inline bool

zones.c

@@ -25,30 +25,30 @@
 	{ FW3_FAMILY_##f, FW3_TABLE_##tbl, FW3_FLAG_##tgt, fmt }
 
 static const struct fw3_chain_spec zone_chains[] = {
-	C(ANY, FILTER, UNSPEC,        "zone_%s_input"),
-	C(ANY, FILTER, UNSPEC,        "zone_%s_output"),
-	C(ANY, FILTER, UNSPEC,        "zone_%s_forward"),
+	C(ANY, FILTER, UNSPEC,        "zone_?_input"),
+	C(ANY, FILTER, UNSPEC,        "zone_?_output"),
+	C(ANY, FILTER, UNSPEC,        "zone_?_forward"),
 
-	C(ANY, FILTER, SRC_ACCEPT,    "zone_%s_src_ACCEPT"),
-	C(ANY, FILTER, SRC_REJECT,    "zone_%s_src_REJECT"),
-	C(ANY, FILTER, SRC_DROP,      "zone_%s_src_DROP"),
+	C(ANY, FILTER, SRC_ACCEPT,    "zone_?_src_ACCEPT"),
+	C(ANY, FILTER, SRC_REJECT,    "zone_?_src_REJECT"),
+	C(ANY, FILTER, SRC_DROP,      "zone_?_src_DROP"),
 
-	C(ANY, FILTER, ACCEPT,        "zone_%s_dest_ACCEPT"),
-	C(ANY, FILTER, REJECT,        "zone_%s_dest_REJECT"),
-	C(ANY, FILTER, DROP,          "zone_%s_dest_DROP"),
+	C(ANY, FILTER, ACCEPT,        "zone_?_dest_ACCEPT"),
+	C(ANY, FILTER, REJECT,        "zone_?_dest_REJECT"),
+	C(ANY, FILTER, DROP,          "zone_?_dest_DROP"),
 
-	C(V4,  NAT,    SNAT,          "zone_%s_postrouting"),
-	C(V4,  NAT,    DNAT,          "zone_%s_prerouting"),
+	C(V4,  NAT,    SNAT,          "zone_?_postrouting"),
+	C(V4,  NAT,    DNAT,          "zone_?_prerouting"),
 
-	C(ANY, RAW,    HELPER,        "zone_%s_helper"),
-	C(ANY, RAW,    NOTRACK,       "zone_%s_notrack"),
+	C(ANY, RAW,    HELPER,        "zone_?_helper"),
+	C(ANY, RAW,    NOTRACK,       "zone_?_notrack"),
 
-	C(ANY, FILTER, CUSTOM_CHAINS, "input_%s_rule"),
-	C(ANY, FILTER, CUSTOM_CHAINS, "output_%s_rule"),
-	C(ANY, FILTER, CUSTOM_CHAINS, "forwarding_%s_rule"),
+	C(ANY, FILTER, CUSTOM_CHAINS, "input_?_rule"),
+	C(ANY, FILTER, CUSTOM_CHAINS, "output_?_rule"),
+	C(ANY, FILTER, CUSTOM_CHAINS, "forwarding_?_rule"),
 
-	C(V4,  NAT,    CUSTOM_CHAINS, "prerouting_%s_rule"),
-	C(V4,  NAT,    CUSTOM_CHAINS, "postrouting_%s_rule"),
+	C(V4,  NAT,    CUSTOM_CHAINS, "prerouting_?_rule"),
+	C(V4,  NAT,    CUSTOM_CHAINS, "postrouting_?_rule"),
 
 	{ }
 };
@@ -327,6 +327,38 @@ fw3_load_zones(struct fw3_state *state, struct uci_package *p)
 }
 
 
+static char *
+format_chain(const char *fmt, const char *zonename)
+{
+	static char chain[32];
+	size_t rem;
+	char *p;
+	int len;
+
+	for (p = chain, rem = sizeof(chain); *fmt; fmt++) {
+		if (*fmt == '?') {
+			len = snprintf(p, rem, "%s", zonename);
+
+			if (len < 0 || len >= rem)
+				break;
+
+			rem -= len;
+			p += len;
+		}
+		else {
+			if (rem <= 1)
+				break;
+
+			*p++ = *fmt;
+			rem--;
+		}
+	}
+
+	*p = 0;
+
+	return chain;
+}
+
 static void
 print_zone_chain(struct fw3_ipt_handle *handle, struct fw3_state *state,
                  bool reload, struct fw3_zone *zone)
@@ -366,7 +398,7 @@ print_zone_chain(struct fw3_ipt_handle *handle, struct fw3_state *state,
 		    !fw3_hasbit(zone->flags[handle->family == FW3_FAMILY_V6], c->flag))
 			continue;
 
-		fw3_ipt_create_chain(handle, reload, c->format, zone->name);
+		fw3_ipt_create_chain(handle, reload, format_chain(c->format, zone->name));
 	}
 
 	if (zone->custom_chains)
@@ -752,7 +784,6 @@ fw3_flush_zones(struct fw3_ipt_handle *handle, struct fw3_state *state,
 {
 	struct fw3_zone *z, *tmp;
 	const struct fw3_chain_spec *c;
-	char chain[32];
 
 	list_for_each_entry_safe(z, tmp, &state->zones, list)
 	{
@@ -775,8 +806,7 @@ fw3_flush_zones(struct fw3_ipt_handle *handle, struct fw3_state *state,
 			if (c->flag && !has(z->flags, handle->family, c->flag))
 				continue;
 
-			snprintf(chain, sizeof(chain), c->format, z->name);
-			fw3_ipt_flush_chain(handle, chain);
+			fw3_ipt_flush_chain(handle, format_chain(c->format, z->name));
 		}
 
 		/* ... then remove the chains */
@@ -791,9 +821,8 @@ fw3_flush_zones(struct fw3_ipt_handle *handle, struct fw3_state *state,
 			if (c->flag && !has(z->flags, handle->family, c->flag))
 				continue;
 
-			snprintf(chain, sizeof(chain), c->format, z->name);
-
-			fw3_ipt_delete_chain(handle, reload, chain);
+			fw3_ipt_delete_chain(handle, reload,
+			                     format_chain(c->format, z->name));
 		}
 
 		del(z->flags, handle->family, handle->table);