bin/62050: [PATCH] Incorrect expansion of "MYADDR" in ppp(8)

[Peter Jeremy]

>Number:         62050
>Category:       bin
>Synopsis:       [PATCH] Incorrect expansion of "MYADDR" in ppp(8)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Jan 28 16:20:11 PST 2004
>Closed-Date:
>Last-Modified:
>Originator:     Peter Jeremy
>Release:        FreeBSD 4.9-RELEASE-p1 i386
>Organization:
Alcatel Australia Limited
>Environment:
System: FreeBSD gsmx07.alcatel.com.au 4.9-RELEASE-p1 FreeBSD 4.9-RELEASE-p1 #0: Fri Dec 19 07:22:50 EST 2003 root at aalp03.alcatel.com.au:/mnt/obj/mnt/rpc/src/sys/gsmx i386

>Description:
	The literal "MYADDR" is incorrectly expanded to the IP address
	associated with the hostname rather than the address defined in
	a previous "set ifaddr" command.

	Code inspection indicates that the faulty code was introduced
	in command.c v1.268 and MFCd in v1.230.2.16

>How-To-Repeat:
	Using the default ppp and ppp.conf from 4.9p1, enter the following
	commands:
gsmx07# ppp
Working in interactive mode
Using interface: tun0
ppp ON gsmx07> set ifaddr 10.1.1.1 10.1.1.2
ppp ON gsmx07> ! echo MYADDR HISADDR
ppp: Pausing until echo finishes
xxx.yyy.xxx.ttt 10.1.1.2
ppp ON gsmx07> 
	(Where xxx.yyy.zzz.ttt is the IP address associated with gsmx07).

	A ppp patched as below responds as expected
gsmx07# /tmp/ppp                         
Working in interactive mode
Using interface: tun0
ppp ON gsmx07> set ifaddr 10.1.1.1 10.1.1.2
ppp ON gsmx07> ! echo MYADDR HISADDR
ppp: Pausing until echo finishes
10.1.1.1 10.1.1.2
ppp ON gsmx07>

>Fix:

	command.c:SetInterfaceAddr() defines "ncpaddr" as a local
	variable and it is unused prior to the code in the patch below.
	ncp->ipcp.cfg.my_range is initialised from the first argument
	to "set ifaddr".  Both ncpaddr_getip4() and ncprange_getaddr()
	copy data (including the source IP address) from the first
	argument to the second argument.  The existing order means that 
	ncpaddr is referenced before it is set.  It is unclear to me
	why using the (undefined) content of ncpaddr results in the
	IP address associated with the hostname being used.

Index: command.c
===================================================================
RCS file: /usr/ncvs/src/usr.sbin/ppp/command.c,v
retrieving revision 1.230.2.20
diff -u -r1.230.2.20 command.c
--- command.c   11 Aug 2003 09:35:50 -0000      1.230.2.20
+++ command.c   28 Jan 2004 02:51:14 -0000
@@ -1576,8 +1576,8 @@
   }
 
   /* 0.0.0.0 means any address (0 bits) */
-  ncpaddr_getip4(&ncpaddr, &ncp->ipcp.my_ip);
   ncprange_getaddr(&ncp->ipcp.cfg.my_range, &ncpaddr);
+  ncpaddr_getip4(&ncpaddr, &ncp->ipcp.my_ip);
   if (ncp->ipcp.my_ip.s_addr == INADDR_ANY)
     ncprange_setwidth(&ncp->ipcp.cfg.my_range, 0);
   bundle_AdjustFilters(arg->bundle, &ncpaddr, NULL);
>Release-Note:
>Audit-Trail:
>Unformatted: