From f62a52b105fdd3aa12cc073b2847140d5b64261f Mon Sep 17 00:00:00 2001 From: Jo-Philipp Wich Date: Wed, 27 May 2020 22:23:23 +0200 Subject: [PATCH] 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 Signed-off-by: Jo-Philipp Wich --- defaults.c | 6 +-- includes.c | 11 ++--- iptables.c | 131 ++++++++++++++++++++++++++++++++------------------- iptables.h | 2 +- options.c | 29 ++++++++---- redirects.c | 28 +++++++---- rules.c | 6 +-- snats.c | 26 ++++++---- utils.c | 21 ++++----- xtables-10.h | 12 ++--- xtables-5.h | 4 +- zones.c | 79 +++++++++++++++++++++---------- 12 files changed, 216 insertions(+), 139 deletions(-) diff --git a/defaults.c b/defaults.c index 6c3ec9d..0580bfc 100644 --- a/defaults.c +++ b/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); diff --git a/includes.c b/includes.c index 23b2244..d1e574c 100644 --- a/includes.c +++ b/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) diff --git a/iptables.c b/iptables.c index 7ad1d22..e7e8b59 100644 --- a/iptables.c +++ b/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) diff --git a/iptables.h b/iptables.h index 402baf2..ca39809 100644 --- a/iptables.h +++ b/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); diff --git a/options.c b/options.c index 7870143..aed0cfb 100644 --- a/options.c +++ b/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; diff --git a/redirects.c b/redirects.c index b928287..775fade 100644 --- a/redirects.c +++ b/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) diff --git a/rules.c b/rules.c index 5230a86..181c6b1 100644 --- a/rules.c +++ b/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); diff --git a/snats.c b/snats.c index 1d78f93..a2706fa 100644 --- a/snats.c +++ b/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); diff --git a/utils.c b/utils.c index da65632..5667bcf 100644 --- a/utils.c +++ b/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) diff --git a/xtables-10.h b/xtables-10.h index 7ea5315..6a2275d 100644 --- a/xtables-10.h +++ b/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 diff --git a/xtables-5.h b/xtables-5.h index 9d11cae..14b54af 100644 --- a/xtables-5.h +++ b/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 diff --git a/zones.c b/zones.c index d915bfd..68b02ab 100644 --- a/zones.c +++ b/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); -- 2.25.1