netdev - Re: [PATCH] Fix IP timestamp option (IPOPT_TS_PRESPEC) handling in ip_options

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1301033472.3890.181.camel@mordor>
Date:	Fri, 25 Mar 2011 07:11:12 +0100
From:	Jan Lübbe <jluebbe@...ian.org>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] Fix IP timestamp option (IPOPT_TS_PRESPEC) handling in
 ip_options_echo()

Hi!

On Thu, 2011-03-24 at 16:20 -0700, David Miller wrote: 
> From: Jan Luebbe <jluebbe@...ian.org>
> Date: Thu, 24 Mar 2011 18:44:22 +0100
> 
> > -					if (soffset + 8 <= optlen) {
> > +					if (soffset + 7 <= optlen) {
> 
> I don't see how you can legally reduce this check from 8 to 7 bytes.
> 
> > +						if (inet_addr_type(dev_net(skb_dst(skb)->dev), addr) != RTN_UNICAST) {
> >  							dopt->ts_needtime = 1;
> >  							soffset += 8;
> >  						}
> 
> Yet keep this code which advances soffset by 8.

The encoding of soffset is a bit unusual, in that the option is 'full'
when soffset = optlen + 1. We need to keep advancing soffset by 8
because that's really the amount of data per entry.

In ip_options_compile, near 'case IPOPT_TS_PRESPEC:' (line 382 in
2.6.38), we have: 
      case IPOPT_TS_PRESPEC:
        if (optptr[2]+7 > optptr[1]) {  
                pp_ptr = optptr + 2;            
                goto error;
        }
        opt->ts = optptr - iph;
        {
                __be32 addr;
                memcpy(&addr, &optptr[optptr[2]-1], 4);
                if (inet_addr_type(net, addr) == RTN_UNICAST)
                        break;
                if (skb)
                        timeptr = (__be32*)&optptr[optptr[2]+3];
        }
        opt->ts_needtime = 1;
        optptr[2] += 8;
        break;

Here optptr[1] matches optlen from _echo and optptr[2] is soffset.

When we use soffset to index into the packet we substract 1. See the
memcpy in my patch which reads from the packet and also the memcpys in
_compile around line 390.

If we checked soffset + 8 <= optlen, we abort updating the timestamp
when exactly space for one entry remains.

Also, I don't think writing to unallocated memory is possible when the
IP header in the echoed packet are doesn't have enough space for the
length indicated by optlen, as we clear dopt with memset(dopt, 0,
sizeof(struct ip_options)) so it dopt must have the right size already.

Best regards,
Jan

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html