bin/104921: ipfw(8) sometimes treat ipv6 input as ipv4 (another variation on PR 91245)

[Stephen Halpin]

>Number:         104921
>Category:       bin
>Synopsis:       ipfw(8) sometimes treat ipv6 input as ipv4 (another variation on PR 91245)
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Mon Oct 30 07:10:19 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator:     Stephen Halpin
>Release:        FreeBSD 6.2-BETA2
>Organization:
None
>Environment:
FreeBSD  6.2-BETA2 FreeBSD 6.2-BETA2 #0: Mon Oct  2 03:32:44 UTC 2006     root at opus.cse.buffalo.edu:/usr/obj/usr/src/sys/SMP  i386

>Description:
Some IPv6 address specifiers are still incorrectly interpreted as IPv4 addresses.  In this particular variation, if a list of addresses starting with an IPv6 host address and followed by either an IPv6 host or network address.  Both source and destination address parsing exhibit this problem.
>How-To-Repeat:
ipfw add allow udp from any to 2000::1,2002::/16

It will result in the error:

ipfw: bad netmask ``:1,2002::/16''

Similarly:

ipfw add allow udp from 2000::1,2002::1 to any

results in the error:

ipfw: bad netmask ``:1,2002::1''

>Fix:
The original proposed fix for 91245 would have caught this, but that fix was changed to use a strrchr to find the last '/' in the address list before passing it to inet_pton(AF_INET6,... to see if it is an IPv6 address.  I'm guessing someone was using the n.n.n.n:m.m.m.m format for multiple IPv4 addresses and they were incorrectly being identified by the original proposed fix as an IPv6 address.  As to why strrchr (reverse search) was used instead of strchr isn't obvious, and if there was a reason, it should be used to evaluate the effectiveness of what I'm proposing.

It's my understanding by looking at this that we want to scan up to the first ',' or the first '/' to find the first IPv6 address without any mask information and pass that to inet_pton(AF_INET6,... to see if it really is an IPv6 address.  To that end, I propose the following:

*** ipfw2.c.start	Mon Oct 30 01:19:40 2006
--- ipfw2.c	Mon Oct 30 01:36:48 2006
***************
*** 3791,3797 ****
  
  	if ((host = strdup(av)) == NULL)
  		return NULL;
! 	if ((ch = strrchr(host, '/')) != NULL)
  		*ch = '\0';
  
  	if (proto == IPPROTO_IPV6  || strcmp(av, "me6") == 0 ||
--- 3791,3797 ----
  
  	if ((host = strdup(av)) == NULL)
  		return NULL;
! 	if ((ch = strpbrk(host, "/,")) != NULL)
  		*ch = '\0';
  
  	if (proto == IPPROTO_IPV6  || strcmp(av, "me6") == 0 ||
***************
*** 3817,3823 ****
  
  	if ((host = strdup(av)) == NULL)
  		return NULL;
! 	if ((ch = strrchr(host, '/')) != NULL)
  		*ch = '\0';
  
  	if (proto == IPPROTO_IPV6  || strcmp(av, "me6") == 0 ||
--- 3817,3823 ----
  
  	if ((host = strdup(av)) == NULL)
  		return NULL;
! 	if ((ch = strpbrk(host, "/,")) != NULL)
  		*ch = '\0';
  
  	if (proto == IPPROTO_IPV6  || strcmp(av, "me6") == 0 ||

>Release-Note:
>Audit-Trail:
>Unformatted: