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>

diff --git a/defaults.c b/defaults.c
index 6c3ec9dd6965a5754d370c3197eb891004ccf1b0..0580bfccccf0e5d4fb9b7704b2f405374cae015d 100644 (file)
--- 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 23b224436f616e469213e2d1573def730350cabf..d1e574c19586a93018842406a4277769ad10f5e4 100644 (file)
--- 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 7ad1d2247ebf17a6f7c7ff54f3f6609e525e6978..e7e8b597c0dd948dbe327e100f1c54710b83bc12 100644 (file)
--- 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 402baf288ff4b0f7138be7e5ef3d4e46354fd5d7..ca39809856cd1f92af1e0fb331785eea84773884 100644 (file)
--- 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 7870143b03f55297466a6540efd3730d38773f49..aed0cfb8a441dcd44ca9c478add57aa49bed20cd 100644 (file)
--- 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 b928287deebb0f8407aa5ecbc4dab75bdda05c66..775fadea86d9cb7168c51d87c49173cbd46884f1 100644 (file)
--- 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 5230a8623fa897f1111cb7f7e7012f06c4acb503..181c6b1a126f4e4f1f7f23f5620bb20d1bd08c6a 100644 (file)
--- 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 1d78f93f30662f475d24bd5817a03c1b106b02ec..a2706faee1004e77550831ca8ef242a75f8e355e 100644 (file)
--- 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 da6563243c0673f8393c16f65116aeb88ba0ea6c..5667bcfd76db5d91191a90e0e705b51d2978745f 100644 (file)
--- 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 7ea5315a45a5551f150e60d63e9bc6b92caf6830..6a2275d276ab62060363d9c6cf566bb4719dc4e6 100644 (file)
--- 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 9d11caeccdf3a0fa808a8f8fb0bd31b3e9bfdc7c..14b54aff06c5fd01a269781a9d1ccb068c11a5b5 100644 (file)
--- 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 d915bfd2ab9a94e490af50d1ff9f8c57601e8e66..68b02abed6a96da98201f51f3935f51394a21870 100644 (file)
--- 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);