[arp] possible DoS, fixes and improvements

[rozhuk.im at gmail.com]

Hi!

> I wonder which version of netinet/if_ether.c you are working from?
uname -a
FreeBSD firewall 9.0-CURRENT FreeBSD 9.0-CURRENT #2: Wed Dec  8 02:53:50
IRKT 2010     root at firewall:/usr/obj/usr/src/sys/RIMx64  amd64
 
--
Rozhuk Ivan
  


> -----Original Message-----
> From: Chuck Swiger [mailto:cswiger at mac.com]
> Sent: Wednesday, December 08, 2010 4:54 AM
> To: Rozhuk.IM at gmail.com
> Cc: freebsd-net at freebsd.org
> Subject: Re: [arp] possible DoS, fixes and improvements
> 
> Hi, Rozhuk--
> 
> On Dec 7, 2010, at 11:19 AM, rozhuk.im at gmail.com wrote:
> > Hi!
> >
> > 1. ah->ar_hln - is depend from ar_hrd?
> > Yes, and for ARPHRD_ETHER is 6 (ETHER_ADDR_LEN)
> > For ARPHRD_IEEE1394 - sizeof(struct fw_hwaddr)
> > ah->ar_hln ignored in ether_output: bcopy(ar_tha(ah), edst,
> ETHER_ADDR_LEN);
> 
> If you know that ar_hrd is ARPHRD_ETHER, then you can either assume the
> length is ETHER_ADDR_LEN, or optionally check it, per RFC 826:
> 
> "When an address resolution packet is received, the receiving
> Ethernet module gives the packet to the Address Resolution module
> which goes through an algorithm similar to the following.
> Negative conditionals indicate an end of processing and a
> discarding of the packet.
> 
> ?Do I have the hardware type in ar$hrd?
> Yes: (almost definitely)
>   [optionally check the hardware length ar$hln]
>   ?Do I speak the protocol in ar$pro?
>   Yes:
>     [optionally check the protocol length ar$pln]"
> 
> > check in in_arpinput:
> > 		if (ifp->if_addrlen != ah->ar_hln) {
> > 			LLE_WUNLOCK(la);
> > 			log(LOG_WARNING,
> > 			    "arp from %*D: addr len: new %d, i/f %d
> (ignored)",
> > 			    ifp->if_addrlen, (u_char *) ar_sha(ah), ":",
> > 			    ah->ar_hln, ifp->if_addrlen);
> > 			goto reply;
> > 		}
> > NO DROP!!!!
> 
> I wonder which version of netinet/if_ether.c you are working from?
> In 7-STABLE sources, it breaks rather than going to generate a reply:
> 
>                         if (ifp->if_addrlen != ah->ar_hln) {
>                                 log(LOG_WARNING,
>                                     "arp from %*D: addr len: "
>                                     "new %d, i/f %d (ignored)",
>                                     ifp->if_addrlen, (u_char *)
> ar_sha(ah),
>                                     ":", ah->ar_hln, ifp->if_addrlen);
>                                 RT_UNLOCK(rt);
>                                 break;
>                         }
> 
> > In reply we get:
> > 		(void)memcpy(ar_tha(ah), ar_sha(ah), ah->ar_hln);
> > 		(void)memcpy(ar_sha(ah), enaddr, ah->ar_hln);
> > Or
> > 			(void)memcpy(ar_tha(ah), ar_sha(ah), ah->ar_hln);
> > 			(void)memcpy(ar_sha(ah), &lle->ll_addr, ah->ar_hln);
> >
> > How to use it see below.
> >
> >
> > 2. ah->ar_pln - does not checked!
> > We can make big arp request (512 nulls after struct arphdr + 2*6 +
> 2*4) ,
> > valid for host, set ar_plt = 255
> > And in reply will receive part of stack or core panic:
> > in_arpinput:
> > (void)memcpy(ar_spa(ah), &itaddr, ah->ar_pln);
> > ...
> > m->m_len = sizeof(*ah) + (2 * ah->ar_pln) + (2 * ah->ar_hln);
> > ( eq arphdr_len(ah) )
> 
> I think I agree that this is not being checked for properly....
> 
> Regards,
> --
> -Chuck