On Thu, May 21, 2015 at 06:42:49PM +0000, Loganaden Velvindron wrote: > Hi All, > > A number of Open Source projects such as OpenSSH, nsd, unbound, > OpenBSD and FreeBSD have implemented a safe replacement for > malloc(x*y) idiom which may lead to overflows. > > Quoting from man page: > Consider calloc() or the extension reallocarray() when there is > multiplication in the size argument of malloc() or realloc(). For > example, avoid this common idiom as it may lead to integer overflow: > > if ((p = malloc(num * size)) == NULL) > err(1, "malloc"); > > A drop-in replacement is the OpenBSD extension reallocarray(): > > if ((p = reallocarray(NULL, num, size)) == NULL) > err(1, "reallocarray"); > > > Replacing malloc() with reallocarray() has led to one vulnerability > being mitigated: > See here: https://www.kb.cert.org/vuls/id/695940 > > The other advantage is that reallocarray() is faster than calloc(x*y) > as there is no zero'ing overhead. > > Patch below with a few conversions to reallocarray(). -- Feedback welcomed ! > Gmail broke the patch. Sorry for that. Sending it using my other account: diff --git a/include/xtables.h b/include/xtables.h index bad11a8..e18b4cd 100644 --- a/include/xtables.h +++ b/include/xtables.h @@ -418,6 +418,7 @@ extern void xtables_init(void); extern void xtables_set_nfproto(uint8_t); extern void *xtables_calloc(size_t, size_t); extern void *xtables_malloc(size_t); +extern void *xtables_reallocarray(void *, size_t, size_t); extern void *xtables_realloc(void *, size_t); extern int xtables_insmod(const char *, const char *, bool); diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c index 8db13b4..8964c1e 100644 --- a/iptables/ip6tables.c +++ b/iptables/ip6tables.c @@ -858,7 +858,7 @@ for_each_chain6(int (*fn)(const xt_chainlabel, int, struct xtc_handle *), chain = ip6tc_next_chain(handle); } - chains = xtables_malloc(sizeof(xt_chainlabel) * chaincount); + chains = xtables_reallocarray(NULL, chaincount, sizeof(xt_chainlabel)); i = 0; chain = ip6tc_first_chain(handle); while (chain) { diff --git a/libxtables/xtables.c b/libxtables/xtables.c index 5357d12..1f62aa3 100644 --- a/libxtables/xtables.c +++ b/libxtables/xtables.c @@ -15,6 +15,23 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ + +/* xtables_reallocarray.c is under: + * Copyright (c) 2008 Otto Moerbeek <otto@xxxxxxxxx> + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ + #include "config.h" #include <ctype.h> #include <errno.h> @@ -308,6 +325,23 @@ void *xtables_malloc(size_t size) return p; } +/* + * This is sqrt(SIZE_MAX+1), as s1*s2 <= SIZE_MAX + * if both s1 < MUL_NO_OVERFLOW and s2 < MUL_NO_OVERFLOW + */ +#define MUL_NO_OVERFLOW ((size_t)1 << (sizeof(size_t) * 4)) + +void * +xtables_reallocarray(void *optr, size_t nmemb, size_t size) +{ + if ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) && + nmemb > 0 && SIZE_MAX / nmemb < size) { + perror("ip[6]tables: reallocarray failed"); + exit(1); + } + return xtables_realloc(optr, size * nmemb); +} + void *xtables_realloc(void *ptr, size_t size) { void *p; @@ -1432,8 +1466,8 @@ void xtables_ipparse_multiple(const char *name, struct in_addr **addrpp, ++loop; /* skip ',' */ } - *addrpp = xtables_malloc(sizeof(struct in_addr) * count); - *maskpp = xtables_malloc(sizeof(struct in_addr) * count); + *addrpp = xtables_reallocarray(NULL, count, sizeof(struct in_addr)); + *maskpp = xtables_reallocarray(NULL, count, sizeof(struct in_addr)); loop = name; @@ -1753,8 +1787,8 @@ xtables_ip6parse_multiple(const char *name, struct in6_addr **addrpp, ++loop; /* skip ',' */ } - *addrpp = xtables_malloc(sizeof(struct in6_addr) * count); - *maskpp = xtables_malloc(sizeof(struct in6_addr) * count); + *addrpp = xtables_reallocarray(NULL, count, sizeof(struct in6_addr)); + *maskpp = xtables_reallocarray(NULL, count, sizeof(struct in6_addr)); loop = name; -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
