On 12/31/2013 06:35 PM, Pablo Neira Ayuso wrote:
On Tue, Dec 31, 2013 at 06:29:52PM +0100, Pablo Neira Ayuso wrote:
On Tue, Dec 31, 2013 at 04:28:39PM +0100, Daniel Borkmann wrote:
Commit 5901b6be885e attempted to introduce IPv6 support into
IRC NAT helper. By doing so, the following code seemed to be removed
by accident:
ip = ntohl(exp->master->tuplehash[IP_CT_DIR_REPLY].tuple.dst.u3.ip);
sprintf(buffer, "%u %u", ip, port);
pr_debug("nf_nat_irc: inserting '%s' == %pI4, port %u\n", buffer, &ip, port);
This leads to the fact that buffer[] was left uninitialized and
contained some stack value. When we call nf_nat_mangle_tcp_packet(),
we call strlen(buffer) on excatly this uninitialized buffer. If we
are unlucky and the skb has enough tailroom, we overwrite resp. leak
contents with values that sit on our stack into the packet and send
that out to the receiver.
Since the rather informal DCC spec [1] does not seem to specify
IPv6 support right now, we log such occurences so that admins can
act accordingly, and drop the packet. I've looked into XChat source,
and IPv6 is not supported there: addresses are in u32 and print
via %u format string.
Therefore, restore old behaviour as in IPv4, use snprintf(), and
log IPv6 packets for now (maybe if there's consensus one day, we
can still add support here). By this, we can safely use strlen(buffer)
in nf_nat_mangle_tcp_packet() and prevent a buffer overflow. Also
simplify some code as we now have ct variable anyway.
[1] http://www.irchelp.org/irchelp/rfc/ctcpspec.html
Thanks Daniel.
No need for red blinking light on...
Oh well, this also affects IPv4. Will pass this to master. Thanks.
Indeed, it affects IPv4 as the current code seems to be not working.
I think for now fixing this should be fine and later on there can be
follow-ups for full support for IPv6; probably that's a completely
different topic / discussion.
Thanks and happy new year,
Daniel
--
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
