Re: 4.1.31 ip_ftp_pxy.c error

Darren Reed <[email protected]>
Newsgroups gmane.comp.security.firewalls.ipfilter
Message-ID <2882_1230475828_49579233_2882_13410_1_4957913B.8010604@reed.wattle.id.au> 
Steve,

I've had another look at this problem with some captured
data from that website and made some changes to the patch.

Please reverse out any prior changes and try these instead.

Darren
ftp.patch (text/plain, 7 KB) 
Index: ip_ftp_pxy.c
===================================================================
RCS file: /devel/CVS/IP-Filter/ip_ftp_pxy.c,v
retrieving revision 2.88.2.26
diff -c -r2.88.2.26 ip_ftp_pxy.c
*** ip_ftp_pxy.c	6 Nov 2008 21:18:33 -0000	2.88.2.26
--- ip_ftp_pxy.c	28 Dec 2008 14:42:54 -0000
***************
*** 35,40 ****
--- 35,45 ----
  #define	FTPXY_PASS_2	14
  #define	FTPXY_PAOK_2	15
  
+ #define	FTPXY_JUNK_OK	0
+ #define	FTPXY_JUNK_BAD	1	/* Ignore all commands for this connection */
+ #define	FTPXY_JUNK_EOL	2	/* consume the rest of this line only */
+ #define	FTPXY_JUNK_CONT	3	/* Saerching for next numeric */
+ 
  /*
   * Values for FTP commands.  Numerics cover 0-999
   */
***************
*** 806,813 ****
  
  	s = buf;
  
! 	if (ftps->ftps_junk == 1)
! 		return 1;
  
  	if (i < 5) {
  		if (ippr_ftp_debug > 3)
--- 811,818 ----
  
  	s = buf;
  
! 	if (ftps->ftps_junk == FTPXY_JUNK_BAD)
! 		return FTPXY_JUNK_BAD;
  
  	if (i < 5) {
  		if (ippr_ftp_debug > 3)
***************
*** 849,855 ****
  			       "ippr_ftp_client_valid",
  			       ftps->ftps_junk, (int)len, (int)i, c,
  			       (int)len, (int)len, buf);
! 		return 1;
  	}
  
  	for (; i; i--) {
--- 854,860 ----
  			       "ippr_ftp_client_valid",
  			       ftps->ftps_junk, (int)len, (int)i, c,
  			       (int)len, (int)len, buf);
! 		return FTPXY_JUNK_BAD;
  	}
  
  	for (; i; i--) {
***************
*** 884,891 ****
  	s = buf;
  	cmd = 0;
  
! 	if (ftps->ftps_junk == 1)
! 		return 1;
  
  	if (i < 5) {
  		if (ippr_ftp_debug > 3)
--- 889,896 ----
  	s = buf;
  	cmd = 0;
  
! 	if (ftps->ftps_junk == FTPXY_JUNK_BAD)
! 		return FTPXY_JUNK_BAD;
  
  	if (i < 5) {
  		if (ippr_ftp_debug > 3)
***************
*** 895,902 ****
  
  	c = *s++;
  	i--;
! 	if (c == ' ')
  		goto search_eol;
  
  	if (ISDIGIT(c)) {
  		cmd = (c - '0') * 100;
--- 900,909 ----
  
  	c = *s++;
  	i--;
! 	if (c == ' ') {
! 		cmd = -1;
  		goto search_eol;
+ 	}
  
  	if (ISDIGIT(c)) {
  		cmd = (c - '0') * 100;
***************
*** 912,917 ****
--- 919,926 ----
  				i--;
  				if ((c != '-') && (c != ' '))
  					goto bad_server_command;
+ 				if (c == '-')
+ 					return FTPXY_JUNK_CONT;
  			} else
  				goto bad_server_command;
  		} else
***************
*** 923,943 ****
  			       "ippr_ftp_server_valid",
  			       ftps->ftps_junk, (int)len, (int)i,
  			       c, (int)len, (int)len, buf);
! 		return 1;
  	}
  search_eol:
  	for (; i; i--) {
  		pc = c;
  		c = *s++;
  		if ((pc == '\r') && (c == '\n')) {
! 			ftps->ftps_cmds = cmd;
! 			return 0;
  		}
  	}
  	if (ippr_ftp_debug > 3)
  		printf("ippr_ftp_server_valid:junk after cmd[%*.*s]\n",
  		       (int)len, (int)len, buf);
! 	return 2;
  }
  
  
--- 932,959 ----
  			       "ippr_ftp_server_valid",
  			       ftps->ftps_junk, (int)len, (int)i,
  			       c, (int)len, (int)len, buf);
! 		if (ftps->ftps_junk == FTPXY_JUNK_CONT)
! 			return FTPXY_JUNK_CONT;
! 		return FTPXY_JUNK_BAD;
  	}
  search_eol:
  	for (; i; i--) {
  		pc = c;
  		c = *s++;
  		if ((pc == '\r') && (c == '\n')) {
! 			if (cmd == -1) {
! 				if (ftps->ftps_junk == FTPXY_JUNK_CONT)
! 					return FTPXY_JUNK_CONT;
! 			} else {
! 				ftps->ftps_cmds = cmd;
! 			}
! 			return FTPXY_JUNK_OK;
  		}
  	}
  	if (ippr_ftp_debug > 3)
  		printf("ippr_ftp_server_valid:junk after cmd[%*.*s]\n",
  		       (int)len, (int)len, buf);
! 	return FTPXY_JUNK_EOL;
  }
  
  
***************
*** 951,957 ****
  	int ret;
  
  	ftps = &ftp->ftp_side[side];
! 
  	if (side == 0)
  		ret = ippr_ftp_client_valid(ftps, buf, len);
  	else
--- 967,973 ----
  	int ret;
  
  	ftps = &ftp->ftp_side[side];
! printf("ippr_ftp_valid(%p,%d,%p,%d)\n", ftp, side, buf, len);
  	if (side == 0)
  		ret = ippr_ftp_client_valid(ftps, buf, len);
  	else
***************
*** 1172,1178 ****
  			       len, len, rptr);
  
  		f->ftps_wptr = wptr;
! 		if (f->ftps_junk != 0) {
  			i = f->ftps_junk;
  			f->ftps_junk = ippr_ftp_valid(ftp, rv, rptr,
  						      wptr - rptr);
--- 1188,1194 ----
  			       len, len, rptr);
  
  		f->ftps_wptr = wptr;
! 		if (f->ftps_junk != FTPXY_JUNK_OK) {
  			i = f->ftps_junk;
  			f->ftps_junk = ippr_ftp_valid(ftp, rv, rptr,
  						      wptr - rptr);
***************
*** 1181,1187 ****
  				printf("%s:junk %d -> %d\n",
  				       "ippr_ftp_process", i, f->ftps_junk);
  
! 			if (f->ftps_junk != 0) {
  				if (wptr - rptr == sizeof(f->ftps_buf)) {
  					if (ippr_ftp_debug > 4)
  						printf("%s:full buffer\n",
--- 1197,1203 ----
  				printf("%s:junk %d -> %d\n",
  				       "ippr_ftp_process", i, f->ftps_junk);
  
! 			if (f->ftps_junk != FTPXY_JUNK_OK) {
  				if (wptr - rptr == sizeof(f->ftps_buf)) {
  					if (ippr_ftp_debug > 4)
  						printf("%s:full buffer\n",
***************
*** 1190,1208 ****
  					f->ftps_wptr = f->ftps_buf;
  					rptr = f->ftps_rptr;
  					wptr = f->ftps_wptr;
- 					/*
- 					 * Because we throw away data here that
- 					 * we would otherwise parse, set the
- 					 * junk flag to indicate just ignore
- 					 * any data upto the next CRLF.
- 					 */
- 					f->ftps_junk = 1;
  					continue;
  				}
  			}
  		}
  
! 		while ((f->ftps_junk == 0) && (wptr > rptr)) {
  			len = wptr - rptr;
  			f->ftps_junk = ippr_ftp_valid(ftp, rv, rptr, len);
  
--- 1206,1217 ----
  					f->ftps_wptr = f->ftps_buf;
  					rptr = f->ftps_rptr;
  					wptr = f->ftps_wptr;
  					continue;
  				}
  			}
  		}
  
! 		while ((f->ftps_junk == FTPXY_JUNK_OK) && (wptr > rptr)) {
  			len = wptr - rptr;
  			f->ftps_junk = ippr_ftp_valid(ftp, rv, rptr, len);
  
***************
*** 1214,1220 ****
  				printf("buf [%*.*s]\n", len, len, rptr);
  			}
  
! 			if (f->ftps_junk == 0) {
  				f->ftps_rptr = rptr;
  				if (rv)
  					inc += ippr_ftp_server(fin, ip, nat,
--- 1223,1229 ----
  				printf("buf [%*.*s]\n", len, len, rptr);
  			}
  
! 			if (f->ftps_junk == FTPXY_JUNK_OK) {
  				f->ftps_rptr = rptr;
  				if (rv)
  					inc += ippr_ftp_server(fin, ip, nat,
***************
*** 1231,1237 ****
  		 * Off to a bad start so lets just forget about using the
  		 * ftp proxy for this connection.
  		 */
! 		if ((f->ftps_cmds == 0) && (f->ftps_junk == 1)) {
  			/* f->ftps_seq[1] += inc; */
  
  			if (ippr_ftp_debug > 1)
--- 1240,1246 ----
  		 * Off to a bad start so lets just forget about using the
  		 * ftp proxy for this connection.
  		 */
! 		if ((f->ftps_cmds == 0) && (f->ftps_junk == FTPXY_JUNK_BAD)) {
  			/* f->ftps_seq[1] += inc; */
  
  			if (ippr_ftp_debug > 1)
***************
*** 1240,1251 ****
  			return APR_ERR(2);
  		}
  
! 		if ((f->ftps_junk != 0) && (rptr < wptr)) {
  			for (s = rptr; s < wptr; s++) {
  				if ((*s == '\r') && (s + 1 < wptr) &&
  				    (*(s + 1) == '\n')) {
  					rptr = s + 2;
! 					f->ftps_junk = 0;
  					break;
  				}
  			}
--- 1249,1261 ----
  			return APR_ERR(2);
  		}
  
! 		if ((f->ftps_junk != FTPXY_JUNK_OK) && (rptr < wptr)) {
  			for (s = rptr; s < wptr; s++) {
  				if ((*s == '\r') && (s + 1 < wptr) &&
  				    (*(s + 1) == '\n')) {
  					rptr = s + 2;
! 					if (f->ftps_junk != FTPXY_JUNK_CONT)
! 						f->ftps_junk = FTPXY_JUNK_OK;
  					break;
  				}
  			}
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.