Re: 答复: [bug-inetutils] telnet (GNU inetutils) 1.7 cmdrc() local buffer overflow hole

[Giuseppe Scrivano]

Hello,

Thanks for your new bug report.  I have amended the fix in the previous
patch and I am going to push it soon.

please keep the address@hidden mailing list CC'ed so others can
follow the discussion.

Cheers,
Giuseppe



>From 888c8157996a4488da6c0ae8b57cca0870a93b6d Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <address@hidden>
Date: Mon, 28 Dec 2009 00:45:49 +0100
Subject: [PATCH] Fix buffer overflows in telnet

---
 ChangeLog         |    8 ++++++++
 bootstrap.conf    |    1 +
 lib/.gitignore    |    5 +++++
 telnet/commands.c |   18 +++++++-----------
 4 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index bcb67d6..9216a04 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2009-12-28  Giuseppe Scrivano  <address@hidden>
+       Reported by: Zhitong Wangzt <address@hidden>
+
+       * bootstrap.conf (gnulib_modules): Add `xvasprintf'.
+       * telnet/commands.c (cmdrc): Alloc `rcname' dinamically.
+       (m1save): Remove.
+       (rcbuf): Remove.
+
 2009-12-19  Alfred M. Szmidt  <address@hidden>
 
        * configure.ac: Bump version number to 1.7.90.
diff --git a/bootstrap.conf b/bootstrap.conf
index ba67bc6..43e7a0c 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -78,6 +78,7 @@ xgetcwd
 xgetdomainname
 xgethostname
 xsize
+xvasprintf
 "
 
 # Read local configuration file
diff --git a/lib/.gitignore b/lib/.gitignore
index c76a7a5..c47b0b9 100644
--- a/lib/.gitignore
+++ b/lib/.gitignore
@@ -18,6 +18,7 @@ argp-version-etc.h
 argp-xinl.c
 argp.h
 asnprintf.c
+asprintf.c
 at-func.c
 basename-lgpl.c
 basename.c
@@ -225,6 +226,7 @@ unlinkat.c
 unlocked-io.h
 vasnprintf.c
 vasnprintf.h
+vasprintf.c
 verify.h
 version-etc-fsf.c
 version-etc.c
@@ -238,6 +240,7 @@ wctype.h
 wctype.in.h
 xalloc-die.c
 xalloc.h
+xasprintf.c
 xgetcwd.c
 xgetcwd.h
 xgetdomainname.c
@@ -248,3 +251,5 @@ xmalloc.c
 xsize.h
 xstrndup.c
 xstrndup.h
+xvasprintf.c
+xvasprintf.h
diff --git a/telnet/commands.c b/telnet/commands.c
index aeb684a..320be85 100644
--- a/telnet/commands.c
+++ b/telnet/commands.c
@@ -97,6 +97,9 @@
 #include "defines.h"
 #include "types.h"
 
+#include "xalloc.h"
+#include "xvasprintf.h"
+
 #if !defined(CRAY) && !defined(sysV88)
 # ifdef HAVE_NETINET_IN_SYSTM_H
 #  include <netinet/in_systm.h>
@@ -3008,7 +3011,6 @@ help (int argc, char *argv[])
 }
 
 static char *rcname = 0;
-static char rcbuf[128];
 
 static void
 cmdrc (char *m1, char *m2)
@@ -3018,23 +3020,17 @@ cmdrc (char *m1, char *m2)
   int gotmachine = 0;
   int l1 = strlen (m1);
   int l2 = strlen (m2);
-  char m1save[64];
 
   if (skiprc)
     return;
 
-  strcpy (m1save, m1);
-  m1 = m1save;
-
   if (rcname == 0)
     {
-      rcname = getenv ("HOME");
-      if (rcname)
-       strcpy (rcbuf, rcname);
+      const char *home = getenv ("HOME");
+      if (home)
+        rcname = xasprintf ("%s/.telnetrc", home);
       else
-       rcbuf[0] = '\0';
-      strcat (rcbuf, "/.telnetrc");
-      rcname = rcbuf;
+        rcname = xstrdup ("/.telnetrc");
     }
 
   if ((rcfile = fopen (rcname, "r")) == 0)
-- 
1.6.5.7





王智通 <address@hidden> writes:

> BTW:
>
> I found commands.c cmdrc()
>
> static void
> cmdrc (char *m1, char *m2)
> {
>   register Command *c;
>   FILE *rcfile;
>   int gotmachine = 0;
>   int l1 = strlen (m1);
>   int l2 = strlen (m2);
>   char m1save[64];
>
>   if (skiprc)
>     return;
>
>   strcpy (m1save, m1);    // It also not check the length of the arg m1.
>   m1 = m1save;
> }
>
> Cmdrc called by tn() also in commands.c
>
> int
> tn (int argc, char *argv[])
> {
>     char *cmd, *hostp = 0, *portp = 0, *user = 0;
>     if (hostp == 0)
>         {
>           hostp = *argv++;
>           --argc;
>           continue;
>         }
>
>   hostname = malloc (strlen (hostp) + 1);
>   if (hostname)
>     strcpy (hostname, hostp);
>
>   /* hostp is passwd to cmdrc(), but in cmdrc, m1salve only has 64 bytes, if 
> there is a site has a long domain,
>     just like 
> www.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com
>     the domain is normal, it can be visit by us.  So i think it will be cause 
> to another buffer overflow hole. */
>   cmdrc (hostp, hostname);
> }
>