Subject: #2 in the reply() and lreply() bogon saga

From: Kev <[email-masked]>
Date: Tue, 07 Oct 1997 13:44:11 EDT

This is in the first lreply() declaration, starting on line 2741 of ftpd.c:

--- Begin code fragment

    VA_START(fmt);

    if (!dolreplies)
        return;

--- End code fragment

Note the missing VA_END before the return.  While this wouldn't be a problem
on most UNIX systems, ANSI C specifies that you MUST always clean up after
a VA_START() (well, va_start(), anyway).  Moreover, both lreply() and the
reply() declaration just before it (hmmm...I'm using "declaration" rather
loosely, aren't I?) have yet another problem.  Basically, the VA_START() and
VA_END() macros should wrap each instance of a v*printf function.

Since I'm rewriting these functions anyway for doing the GSSAPI authentication
stuff, what I've planned on doing is simply ripping the existing code out
and replacing it with two functions, reply() and lreply(), that call out to
a single function, vreply(), which builds the numeric string, does encryption
if necessary, does the debugging syslog, and returns.  If you desire (which
I presume you will), I can submit a patch which does the proper replacement
as soon as I get to the point where it's been tested and proven to have
some reasonable pretext of working without bugs :)
-- 
Kevin L. Mitchell                                             [email-masked]
-----------------------------------------------------------------------------
MIT Kerberos Development Team                            Work: (617) 253-9483
http://web.mit.edu/klmitch/www/               PGP keys available upon request

---

From: [email-masked] (Stan Barber)
Date: Tue, 7 Oct 1997 12:46:51 CDT

> some reasonable pretext of working without bugs :)

Great!

-- 
Stan   | Academ Consulting Services        |internet: [email-masked]
Olan   | For more info on academ, see this |uucp: {mcsun|amdahl}!academ!sob
Barber | URL- http://www.academ.com/academ |Opinions expressed are only mine.

---

From: Kev <[email-masked]>
Date: Fri, 31 Oct 1997 16:15:42 EST

> > some reasonable pretext of working without bugs :)

I've been lame, but here, finally, is the promised patch, in unified
format (if you request another format, I can be convinced to deal).
Cursory testing has been performed; lreply(), reply(), and autospout
support all seem to work as expected.

------ Patch begins ------
--- new/wu-ftpd-2.4.2-beta-15/src/ftpd.c	Fri Sep 12 11:42:46 1997
+++ reply/wu-ftpd/src/ftpd.c	Fri Oct 31 16:09:39 1997
@@ -2685,145 +2685,112 @@
     /* NOTREACHED */
 }
 
+#define USE_REPLY_NOTFMT	(1<<30)	/* fmt is not a printf fmt (KLUDGE) */
+#define USE_REPLY_LONG		(1<<31)	/* this is a long reply; use a - */
+
 void
-#if defined (HAVE_VPRINTF)
-/* VARARGS2 */
 #ifdef __STDC__
-reply(int n, char *fmt, ...)
+vreply(long flags, int n, char *fmt, va_list ap)
 #else
-reply(n, fmt, va_alist)
+vreply(flags, n, fmt, ap)
+     long flags;
      int n;
      char * fmt;
-     va_dcl
+     va_list ap;
 #endif
 {
-    VA_LOCAL_DECL
-    
-    VA_START(fmt); 
+  char buf[BUFSIZ];
 
-    if (autospout != NULL) {
-        char *ptr = autospout;
+  flags &= USE_REPLY_NOTFMT | USE_REPLY_LONG;
 
-        printf("%d-", n);
-        while (*ptr) {
-            if (*ptr == '\n') {
-                fputs("\r\n", stdout);
-                if (*(++ptr))
-                    printf("%03d-", n);
-            } else {
-                putc(*ptr++,stdout);
-            }
-        }
-        if (*(--ptr) != '\n')
-            printf("\r\n");
-        if (autospout_free) {
-            (void) free(autospout);
-            autospout_free = 0;
-        }
-        autospout = 0;
-    }
-    printf("%d ", n);
-    vprintf(fmt, ap);
-    printf("\r\n");
-    (void) fflush(stdout);
+  if (n) /* if numeric is 0, don't output one; use n==0 in place of printf's */
+    sprintf(buf, "%d%c", n, flags & USE_REPLY_LONG ? '-' : ' ');
 
-    if (debug) {
-        char buf[BUFSIZ];
-        (void) vsprintf(buf, fmt, ap);
+  /* This is somewhat of a kludge for autospout.  I personally think that
+   * autospout should be done differently, but that's not my department. -Kev
+   */
+  if (flags & USE_REPLY_NOTFMT)
+    sprintf(buf + (n ? 4 : 0), "%s", fmt);
+  else
+    vsprintf(buf + (n ? 4 : 0), fmt, ap);
 
-        syslog(LOG_DEBUG, "<--- %d ", n);
-        syslog(LOG_DEBUG, "%s", buf);
-    }
+  if (debug) /* debugging output :) */
+    syslog(LOG_DEBUG, "<--- %s", buf);
 
-    VA_END;
+  /* Yes, you want the debugging output before the client output; wrapping
+   * stuff goes here, you see, and you want to log the cleartext and send
+   * the wrapped text to the client.
+   */
+
+  printf("%s\r\n", buf); /* and send it to the client */
+  fflush(stdout);
 }
 
 void
-/* VARARGS2 */
 #ifdef __STDC__
-lreply(int n, char *fmt,...)
+reply(int n, char *fmt, ...)
 #else
-lreply(n, fmt, va_alist)
+reply(n, fmt, va_alist)
      int n;
      char * fmt;
      va_dcl
 #endif
 {
-    VA_LOCAL_DECL
+  VA_LOCAL_DECL
 
-    VA_START(fmt);
+  if (autospout != NULL) { /* deal with the autospout stuff... */
+    char *p, *ptr = autospout;
 
-    if (!dolreplies)
-        return;
-    printf("%d-", n);
-    vprintf(fmt, ap);
-    printf("\r\n");
-    (void) fflush(stdout);
+    while (*ptr) {
+      if ((p = strchr(ptr, '\n')) != NULL) /* step through line by line */
+	*p = '\0';
 
-    if (debug) {
-        char buf[BUFSIZ];
-        (void) vsprintf(buf, fmt, ap);
+      /* send a line...(note that this overrides dolreplies!) */
+      vreply(USE_REPLY_LONG | USE_REPLY_NOTFMT, n, ptr, 0);
 
-        syslog(LOG_DEBUG, "<--- %d- ", n);
-        syslog(LOG_DEBUG, "%s",buf);
+      if (p)
+	ptr = p + 1; /* set to the next line... (\0 is handled in the while) */
+      else
+	break; /* oh, we're done; drop out of the loop */
     }
 
-    VA_END;
-}
+    if (autospout_free) { /* free autospout if necessary */
+      (void) free(autospout);
+      autospout_free = 0;
+    }
+    autospout = 0; /* clear the autospout */
+  }
 
-#else
-/* VARARGS2 */
-void
-reply(int n, char *fmt, int p0, int p1, int p2, int p3, int p4, int p5)
-{
-    if (autospout != NULL) {
-        char *ptr = autospout;
+  VA_START(fmt);
 
-        printf("%d-", n);
-        while (*ptr) {
-            if (*ptr == '\n') {
-                printf("\r\n");
-                if (*(++ptr))
-                    printf("%d-", n);
-            } else {
-                putc(*ptr++,stdout);
-            }
-        }
-        if (*(--ptr) != '\n')
-            printf("\r\n");
-        if (autospout_free) {
-            (void) free(autospout);
-            autospout_free = 0;
-        }
-        autospout = 0;
-    }
-    printf("%d ", n);
-    printf(fmt, p0, p1, p2, p3, p4, p5);
-    printf("\r\n");
-    (void) fflush(stdout);
-    if (debug) {
-        syslog(LOG_DEBUG, "<--- %d ", n);
-        syslog(LOG_DEBUG, fmt, p0, p1, p2, p3, p4, p5);
-    }
+  /* send the reply */
+  vreply(0, n, fmt, ap);
+
+  VA_END;
 }
 
 void
-/* VARARGS2 */
-void
-lreply(int n, char *fmt, int p0, int p1, int p2, int p3, int p4, int p5)
+#ifdef __STDC__
+lreply(int n, char *fmt, ...)
+#else
+lreply(n, fmt, va_alist)
+     int n;
+     char * fmt;
+     va_dcl
+#endif
 {
-    if (!dolreplies)
-        return;
-    printf("%d-", n);
-    printf(fmt, p0, p1, p2, p3, p4, p5);
-    printf("\r\n");
-    (void) fflush(stdout);
-    if (debug) {
-        syslog(LOG_DEBUG, "<--- %d- ", n);
-        syslog(LOG_DEBUG, fmt, p0, p1, p2, p3, p4, p5);
-    }
+  VA_LOCAL_DECL
+
+  if (!dolreplies) /* prohibited from doing long replies? */
+    return;
+
+  VA_START(fmt);
+
+  /* send the reply */
+  vreply(USE_REPLY_LONG, n, fmt, ap);
+
+  VA_END;
 }
-#endif
 
 void
 #ifdef __STDC__