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.
--
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
