Date:         Sat, 12 Aug 2000 15:38:26 -0700
From: Mike Schiffman <michael.schiffman@GUARDENT.COM>
Subject:      Remote vulnerability in Gopherd 2.x patch redux
To: BUGTRAQ@SECURITYFOCUS.COM

The workaround patch included in advisory A0208102000 is flawed.  Unfortunately
this was not noticed until just after the advisory was posted.  The original
patch made use of strncpy (which is not guaranteed to NUL terminate the
resulting string) and it also passed in a signed length to strncat (which takes
an unsigned length parameter).  Due to these two flaws, the opportunity for
overflowing the destination buffer still existed.  The following replacement
patch fixes these two issues (as well as the original buffer flow).  We
apologize for any inconvenience this mistake may have caused, and would like to
thank all those who noticed these flaws and gave us feedback.


2.3 patch:

diff -ru gopher2_3.old/gopherd/authenticate.c gopher2_3/gopherd/authenticate.c
--- gopher2_3.old/gopherd/authenticate.c	Sat Aug 12 16:34:47 2000
+++ gopher2_3/gopherd/authenticate.c	Sat Aug 12 16:51:51 2000
@@ -494,11 +494,12 @@
      char          keystr[256];
      char         *cp;
      Desnum        c;
-     int i;
+     int i, keysize;

-     strcpy(keystr, user);
-     strcat(keystr, ip);
-     strcat(keystr, key);
+     keysize = sizeof(keystr)-1, memset(keystr, 0, keysize+1);
+     strncat(keystr, user, keysize), i = keysize - strlen(keystr);
+     strncat(keystr, ip, i), i = keysize - strlen(keystr);
+     strncat(keystr, key, i);

      Debug("Encoding key %s\n", keystr);


2.3.1 patch:

diff -ru gopher2_3.1.old/gopherd/authenticate.c
gopher2_3.1/gopherd/authenticate.c
--- gopher2_3.1.old/gopherd/authenticate.c	Sat Aug 12 16:34:57 2000
+++ gopher2_3.1/gopherd/authenticate.c	Sat Aug 12 16:51:40 2000
@@ -496,13 +496,10 @@
      Desnum        c;
      int i, keysize;

-/*     strcpy(keystr, user);
-     strcat(keystr, ip);
-     strcat(keystr, key); */
-	i = keysize = sizeof(keystr)-1;
-	strncpy(keystr, user, i), i -= strlen(keystr);
-	strncat(keystr, ip, i), i -= strlen(keystr);
-	strncat(keystr, key, i), keystr[keysize] = '\0';
+     keysize = sizeof(keystr)-1, memset(keystr, 0, keysize+1);
+     strncat(keystr, user, keysize), i = keysize - strlen(keystr);
+     strncat(keystr, ip, i), i = keysize - strlen(keystr);
+     strncat(keystr, key, i);

      Debug("Encoding key %s\n", keystr);


--
Mike D. Schiffman
Director of Research and Development
Guardent, Inc.
http://www.guardent.com