Subject: Re: kern/29582: panic on NetBSD 2.0_STABLE bridge with ipnat ftp proxy
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Jukka Salmi <j+nbsd@2005.salmi.ch>
List: netbsd-bugs
Date: 05/18/2005 23:25:02
The following reply was made to PR kern/29582; it has been noted by GNATS.

From: Jukka Salmi <j+nbsd@2005.salmi.ch>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/29582: panic on NetBSD 2.0_STABLE bridge with ipnat ftp proxy
Date: Thu, 19 May 2005 01:24:41 +0200

 --r5Pyd7+fXNt84Ff3
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 I just tried to reproduce the problem using todays netbsd-2 sources -
 the bug still exists. Furthermore I applied a patch originally from
 Darren Reed, sent to me by Pavel Cahyna and modified by me to cleanly
 apply to src/sys/dist/ipf/netinet/ip_ftp_pxy.c revision 1.4.8.1 - the
 resulting patch is attached to this message. It did _not_ fix the
 problem; it made active mode FTP fail as if no proxy would be running.
 I didn't investigate further. I possibly made a mistake while applying
 it...
 
 
 Regards, Jukka
 
 -- 
 bashian roulette:
 $ ((RANDOM%6)) || rm -rf ~
 
 --r5Pyd7+fXNt84Ff3
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename="ip_ftp_pxy.c.patch_unidiff"
 
 --- sys/dist/ipf/netinet/ip_ftp_pxy.c.orig	2005-02-06 08:40:56.000000000 +0100
 +++ sys/dist/ipf/netinet/ip_ftp_pxy.c	2005-05-19 00:49:36.000000000 +0200
 @@ -253,6 +253,17 @@
  #endif
  		return 0;
  	}
 +	sp = a5 << 8 | a6;
 +	/*
 +	 * Don't allow the PORT command to specify a port < 1024 due to
 +	 * security crap.
 +	 */
 +	if (sp < 1024) {
 +#if !defined(_KERNEL) || defined(IPF_FTP_DEBUG)
 +		printf("ippr_ftp_port:sp(%d) < 1024\n", sp);
 +#endif
 +		return 0;
 +	}
  
  #if !defined(_KERNEL)
  	bcopy(newbuf, MTOD(m, char *) + off, nlen);
 @@ -260,16 +271,16 @@
  # if defined(MENTAT)
  	if (inc < 0)
  		(void)adjmsg(m, inc);
 -# else
 +# else /* defined(MENTAT) */
 +	/*
 +	 * m_adj takes care of pkthdr.len, if required and treats inc<0 to
 +	 * mean remove -len bytes from the end of the packet.
 +	 * The mbuf chain will be extended if necessary by m_copyback().
 +	 */
  	if (inc < 0)
  		m_adj(m, inc);
 -#  ifdef	M_PKTHDR
 -	if (!(m->m_flags & M_PKTHDR))
 -		m->m_pkthdr.len += inc;
 -#  endif
 -# endif
 -#endif
 -	/* the mbuf chain will be extended if necessary by m_copyback() */
 +# endif /* defined(MENTAT) */
 +#endif /* !defined(_KERNEL) */
  	COPYBACK(m, off, nlen, newbuf);
  
  	if (inc != 0) {
 @@ -279,21 +290,6 @@
  	}
  
  	/*
 -	 * Add skeleton NAT entry for connection which will come back the
 -	 * other way.
 -	 */
 -	sp = a5 << 8 | a6;
 -	/*
 -	 * Don't allow the PORT command to specify a port < 1024 due to
 -	 * security crap.
 -	 */
 -	if (sp < 1024) {
 -#if !defined(_KERNEL) || defined(IPF_FTP_DEBUG)
 -		printf("ippr_ftp_port:sp(%d) < 1024\n", sp);
 -#endif
 -		return 0;
 -	}
 -	/*
  	 * The server may not make the connection back from port 20, but
  	 * it is the most likely so use it here to check for a conflicting
  	 * mapping.
 @@ -302,6 +298,10 @@
  	fi.fin_flx |= FI_IGNORE;
  	fi.fin_data[0] = sp;
  	fi.fin_data[1] = fin->fin_data[1] - 1;
 +	/*
 +	 * Add skeleton NAT entry for connection which will come back the
 +	 * other way.
 +	 */
  	if (nat->nat_dir == NAT_OUTBOUND)
  		nat2 = nat_outlookup(&fi, NAT_SEARCH|IPN_TCP, nat->nat_p,
  				     nat->nat_inip, nat->nat_oip);
 @@ -617,15 +617,19 @@
  	}
  
  #if !defined(_KERNEL)
 -	bcopy(newmsg, (char *)m + off, nlen);
 +	bcopy(newmsg, MTOD(m, char *) + off, nlen);
  #else
  # if defined(MENTAT)
  	if (inc < 0)
  		(void)adjmsg(m, inc);
  # else /* defined(MENTAT) */
 +	/*
 +	 * m_adj takes care of pkthdr.len, if required and treats inc<0 to
 +	 * mean remove -len bytes from the end of the packet.
 +	 * The mbuf chain will be extended if necessary by m_copyback().
 +	 */
  	if (inc < 0)
  		m_adj(m, inc);
 -	/* the mbuf chain will be extended if necessary by m_copyback() */
  # endif /* defined(MENTAT) */
  #endif /* !defined(_KERNEL) */
  	COPYBACK(m, off, nlen, newmsg);
 @@ -795,19 +799,30 @@
  char *buf;
  size_t len;
  {
 -	register char *s, c;
 +	register char *s, c, pc;
  	register size_t i = len;
  	char cmd[5];
  
 +	s = buf;
 +	if (ftps->ftps_junk == 2) {
 +		for (; i > 0; i--) {
 +			c = *s++;
 +			if ((c == '\r') && (i > 1) && (*(s + 1) == '\n'))
 +				return 1;
 +			pc = c;
 +		}
 +		return 2;
 +	}
 +
  	if (i < 5) {
  #if !defined(_KERNEL) || defined(IPF_FTP_DEBUG)
  		printf("ippr_ftp_client_valid:i(%d) < 5\n", (int)i);
  #endif
  		return 2;
  	}
 -	s = buf;
 -	c = *s++;
 +
  	i--;
 +	c = *s++;
  
  	if (ISALPHA(c)) {
  		cmd[0] = TOUPPER(c);
 @@ -844,7 +859,7 @@
  
  	for (; i; i--) {
  		c = *s++;
 -		if (c == '\n') {
 +		if ((c == '\r') && (i > 1) && (*(s + 1) == '\n')) {
  			cmd[4] = '\0';
  			if (!strcmp(cmd, "PASV"))
  				ftps->ftps_cmds = FTPXY_C_PASV;
 @@ -876,6 +891,15 @@
  	cmd = 0;
  	i--;
  
 +	if (ftps->ftps_junk == 2) {
 +		for (; i; i--) {
 +			c = *s++;
 +			if ((c == '\r') && (i > 1) && (*(s + 1) == '\n'))
 +				return 1;
 +		}
 +		return 2;
 +	}
 +
  	if (c == ' ')
  		goto search_eol;
  
 @@ -908,7 +932,7 @@
  search_eol:
  	for (; i; i--) {
  		c = *s++;
 -		if (c == '\n') {
 +		if ((c == '\r') && (i > 1) && (*(s + 1) == '\n')) {
  			ftps->ftps_cmds = cmd;
  			return 0;
  		}
 
 --r5Pyd7+fXNt84Ff3--