mnl: catch bogus expressions before crashing

commit d3339f9e35ee4dddf290fcc3e9cc63dac8cb836a upstream.

We can't recover from errors here, but we can abort with a more
precise reason than 'segmentation fault', or stack corruptions
that get caught way later, or not at all.

expr->value is going to be read, we can't cope with other expression
types here.

We will copy to stack buffer of IFNAMSIZ size, abort if we would
overflow.

Check there is a NUL byte present too.
This is a preemptive patch, I've seen one crash in this area but
no reproducer yet.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/src/mnl.c b/src/mnl.c
index 731426da8fddd9bc623807fced9a5bd09b87831c..64c7ec91dd17f0e4359d1555d584fb5703c86435 100644 (file)
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -707,9 +707,20 @@ static void nft_dev_add(struct nft_dev *dev_array, const struct expr *expr, int
        unsigned int ifname_len;
        char ifname[IFNAMSIZ];
 
+       if (expr->etype != EXPR_VALUE)
+               BUG("Must be a value, not %s\n", expr_name(expr));
+
        ifname_len = div_round_up(expr->len, BITS_PER_BYTE);
        memset(ifname, 0, sizeof(ifname));
+
+       if (ifname_len > sizeof(ifname))
+               BUG("Interface length %u exceeds limit\n", ifname_len);
+
        mpz_export_data(ifname, expr->value, BYTEORDER_HOST_ENDIAN, ifname_len);
+
+       if (strnlen(ifname, IFNAMSIZ) >= IFNAMSIZ)
+               BUG("Interface length %zu exceeds limit, no NUL byte\n", strnlen(ifname, IFNAMSIZ));
+
        dev_array[i].ifname = xstrdup(ifname);
        dev_array[i].location = &expr->location;
 }