>Number:         23435
>Category:       bin
>Synopsis:       [PATCH] ftp and ftpd has broken ASCII transfer type implementation
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Nov 14 11:28:01 UTC 2003
>Closed-Date:
>Last-Modified:
>Originator:     Andrey Beresovsky
>Release:        FreeBSD 5.1-CURRENT i386
>Organization:
Rostov State University
>Environment:
System: FreeBSD 5.1-CURRENT #2: Fri Nov 7 22:15:49 MSK 2003 i386


	
>Description:
According to RFC 959 (FILE TRANSFER PROTOCOL) for ASCII transfer type "the <CRLF> sequence 
should be used where necessary to denote the end of a line of text" (Chapter 3.1.1.1.).

NetBSD's ftp client and ftpd server implement ASCII transfer type by inserting <CR> before each <LF>
without checking that may be <CR> allready exists. It leads to <CR><CR><LF> sequences
in files which allready had <CR><LF> end of lines (ex.: text files from Windows systems).

Attached patches should fix this problem.

>How-To-Repeat:
1. Create a text file with <CRLF> end of lines.
For ftp:
2. Send this file to ftpd server using NetBSD's ftp program (ASCII transfer type).
3. You'll get a file with <CR><CR><LF> EOLs on windows and <CR><LF> on unixes.
For ftpd:
2. Receive (GET) this file from NetBSD's ftpd server.
3. Same as 3. for ftp program.

>Fix:
lukemftp (ftp.c rev. 1.122):
--- ftp.c.orig	Thu Nov 13 21:51:56 2003
+++ ftp.c	Thu Nov 13 22:44:28 2003
@@ -835,6 +835,7 @@
 		break;
 
 	case TYPE_A:
+		d = '\0'; /* using to store previous character */
 		while ((c = getc(fin)) != EOF) {
 			if (c == '\n') {
 				while (hash && (!progress || filesize < 0) &&
@@ -843,19 +844,16 @@
 					(void)fflush(ttyout);
 					hashbytes += mark;
 				}
-				if (ferror(dout))
-					break;
-				(void)putc('\r', dout);
-				bytes++;
+				if (d != '\r') {
+					if (ferror(dout))
+						break;
+					(void)putc('\r', dout);
+					bytes++;
+				}
 			}
 			(void)putc(c, dout);
 			bytes++;
-#if 0	/* this violates RFC */
-			if (c == '\r') {
-				(void)putc('\0', dout);
-				bytes++;
-			}
-#endif
+			d = c;
 		}
 		if (hash && (!progress || filesize < 0)) {
 			if (bytes < hashbytes)

lukemftpd (ftpd.c rev. 1.150):
--- ftpd.c.orig	Thu Nov 13 23:16:54 2003
+++ ftpd.c	Thu Nov 13 23:22:36 2003
@@ -1918,7 +1918,8 @@
 static int
 send_data(FILE *instr, FILE *outstr, const struct stat *st, int isdata)
 {
-	int	 c, filefd, netfd, rval;
+	int c, filefd, netfd, rval;
+	int d = '\0';
 
 	transflag = 1;
 	rval = -1;
@@ -1933,9 +1935,11 @@
 		while ((c = getc(instr)) != EOF) {
 			byte_count++;
 			if (c == '\n') {
-				if (ferror(outstr))
-					goto data_err;
-				(void) putc('\r', outstr);
+				if (d != '\r') {
+					if (ferror(outstr))
+						goto data_err;
+					(void) putc('\r', outstr);
+				}
 				if (isdata) {
 					total_data_out++;
 					total_data++;
@@ -1952,6 +1956,7 @@
 			total_bytes++;
 			if ((byte_count % 4096) == 0)
 				(void) alarm(curclass.timeout);
+			d = c;
 		}
 		(void) alarm(0);
 		fflush(outstr);
>Release-Note:
>Audit-Trail:
>Unformatted: