Date: Sun, 11 May 2003 18:43:43 +0400 (MSD) From: Alex Semenyaka <alexs@ratmir.ru> To: FreeBSD-gnats-submit@FreeBSD.org Subject: bin/52072: Wrong behaviour of the ftpd when the OOB data received Message-ID: <200305111443.h4BEhh3m015012@snark.rinet.ru> Resent-Message-ID: <200305111450.h4BEo9sM096560@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 52072
>Category: bin
>Synopsis: Wrong behaviour of the ftpd when the OOB data received
>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: Sun May 11 07:50:09 PDT 2003
>Closed-Date:
>Last-Modified:
>Originator: Alex Semenyaka
>Release: FreeBSD 4.8-RC i386
>Organization:
Ratmir
>Environment:
System: FreeBSD snark.rinet.ru 4.8-RC FreeBSD 4.8-RC #1: Tue Mar 4 14:49:11 MSK 2003 root@snark.rinet.ru:/usr/obj/usr/src/sys/SNARK i386
>Description:
Some time ago from OpenBSD were moved to FreeBSD ftpd some changes, concerning
the treatment of the OOB data. Now ftpd set the flag recvurg when it receives
them:
a.sa_handler = sigurg;
sa.sa_flags = 0; /* don't restart syscalls for SIGURG */
(void)sigaction(SIGURG, &sa, NULL);
[...some code...]
static void
sigurg(signo)
int signo;
{
recvurg = 1;
}
Then during the transfer this flag is checking and if it is set, transfer is
aborted:
if (recvurg)
goto got_oob;
[...some code...]
got_oob:
myoob();
recvurg = 0;
transflag = 0;
return (-1);
Now, OOB data is the way to send ABRT or STAT message. In case of ABRT that
behaviour is perfect: transfer should be stopped immediately. But in the case
of STAT we should continue.
I marked the problem as non-critical since the current ftp implementations
(incluing native ftp, lukemftp, ncftp) to the best of my knowledge do not
use OOB data only to send ABRT, so this is only potentially wrong behaviour.
However that should be fixed I beleive in order to hadle future possible
situation if some client will use OOB data for the STAT request.
>How-To-Repeat:
Built into ftp(1) the possibility to send OOB data to request STAT info and
observe the abort of the connection.
>Fix:
--- ftpd.c.old Sun May 11 18:05:55 2003
+++ ftpd.c Sun May 11 18:41:50 2003
@@ -247,5 +247,5 @@
static void ack __P((char *));
static void sigurg __P((int));
-static void myoob __P((void));
+static int myoob __P((void));
static int checkuser __P((char *, char *, int, char **));
static FILE *dataconn __P((char *, off_t, char *));
@@ -1970,5 +1970,5 @@
int isreg;
{
- int c, filefd, netfd;
+ int c, filefd, netfd, is_abrt;
char *buf;
off_t cnt;
@@ -1979,6 +1979,12 @@
case TYPE_A:
while ((c = getc(instr)) != EOF) {
- if (recvurg)
- goto got_oob;
+ if (recvurg) {
+ is_abrt = myoob();
+ recvurg = 0;
+ if (is_abrt) {
+ transflag = 0;
+ return (-1);
+ }
+ }
byte_count++;
if (c == '\n') {
@@ -1989,6 +1995,12 @@
(void) putc(c, outstr);
}
- if (recvurg)
- goto got_oob;
+ if (recvurg) {
+ is_abrt = myoob();
+ recvurg = 0;
+ if (is_abrt) {
+ transflag = 0;
+ return (-1);
+ }
+ }
fflush(outstr);
transflag = 0;
@@ -2024,6 +2036,12 @@
*/
byte_count += cnt;
- if (recvurg)
- goto got_oob;
+ if (recvurg) {
+ is_abrt = myoob();
+ recvurg = 0;
+ if (is_abrt) {
+ transflag = 0;
+ return (-1);
+ }
+ }
offset += cnt;
filesize -= cnt;
@@ -2077,9 +2095,4 @@
return (-1);
-got_oob:
- myoob();
- recvurg = 0;
- transflag = 0;
- return (-1);
}
@@ -2095,5 +2108,5 @@
{
int c;
- int cnt, bare_lfs;
+ int cnt, bare_lfs, is_abrt;
char buf[BUFSIZ];
@@ -2106,12 +2119,24 @@
case TYPE_L:
while ((cnt = read(fileno(instr), buf, sizeof(buf))) > 0) {
- if (recvurg)
- goto got_oob;
+ if (recvurg) {
+ is_abrt = myoob();
+ recvurg = 0;
+ if (is_abrt) {
+ transflag = 0;
+ return (-1);
+ }
+ }
if (write(fileno(outstr), buf, cnt) != cnt)
goto file_err;
byte_count += cnt;
}
- if (recvurg)
- goto got_oob;
+ if (recvurg) {
+ is_abrt = myoob();
+ recvurg = 0;
+ if (is_abrt) {
+ transflag = 0;
+ return (-1);
+ }
+ }
if (cnt < 0)
goto data_err;
@@ -2126,6 +2151,12 @@
case TYPE_A:
while ((c = getc(instr)) != EOF) {
- if (recvurg)
- goto got_oob;
+ if (recvurg) {
+ is_abrt = myoob();
+ recvurg = 0;
+ if (is_abrt) {
+ transflag = 0;
+ return (-1);
+ }
+ }
byte_count++;
if (c == '\n')
@@ -2143,6 +2174,12 @@
contin2: ;
}
- if (recvurg)
- goto got_oob;
+ if (recvurg) {
+ is_abrt = myoob();
+ recvurg = 0;
+ if (is_abrt) {
+ transflag = 0;
+ return (-1);
+ }
+ }
fflush(outstr);
if (ferror(instr))
@@ -2174,9 +2211,4 @@
return (-1);
-got_oob:
- myoob();
- recvurg = 0;
- transflag = 0;
- return (-1);
}
@@ -2633,12 +2665,15 @@
}
-static void
+/* Returns non-zero in the case of ABRT */
+
+static int
myoob()
{
char *cp;
+ int is_abrt = 0;
/* only process if transfer occurring */
if (!transflag)
- return;
+ return is_abrt;
cp = tmpline;
if (getline(cp, 7, stdin) == NULL) {
@@ -2651,4 +2686,5 @@
reply(426, "Transfer aborted. Data connection closed.");
reply(226, "Abort successful");
+ is_abrt = 1;
}
if (strcmp(cp, "STAT\r\n") == 0) {
@@ -2660,4 +2696,5 @@
reply(213, "Status: %qd bytes transferred", byte_count);
}
+ return is_abrt;
}
@@ -2979,4 +3016,5 @@
int freeglob = 0;
glob_t gl;
+ int is_abrt;
if (strpbrk(whichf, "~{[*?") != NULL) {
@@ -3044,8 +3082,10 @@
if (recvurg) {
- myoob();
+ is_abrt = myoob();
recvurg = 0;
- transflag = 0;
- goto out;
+ if (is_abrt) {
+ transflag = 0;
+ goto out;
+ }
}
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200305111443.h4BEhh3m015012>