Debian Bug report logs - #264846
telnet: Buffer Overrun by unchecked environment variables

version graph

Package: telnet; Maintainer for telnet is Alberto Gonzalez Iniesta <agi@inittab.org>; Source for telnet is src:netkit-telnet.

Reported by: Josh Martin <jmartin@columbiaservices.net>

Date: Tue, 10 Aug 2004 17:18:04 UTC

Severity: normal

Tags: patch

Found in version 0.17-24

Fixed in version netkit-telnet/0.17-25

Done: Robert Millan <rmh@debian.org>

Bug is archived. No further changes may be made.

Toggle useless messages

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to debian-bugs-dist@lists.debian.org, Robert Millan <rmh@debian.org>:
Bug#264846; Package telnet. Full text and rfc822 format available.

Acknowledgement sent to Josh Martin <jmartin@columbiaservices.net>:
New Bug report received and forwarded. Copy sent to Robert Millan <rmh@debian.org>. Full text and rfc822 format available.

Message #5 received at submit@bugs.debian.org (full text, mbox):

From: Josh Martin <jmartin@columbiaservices.net>
To: bugs@debian.org
Subject: telnet: Buffer Overrun by unchecked environment variables
Date: Tue, 10 Aug 2004 10:12:06 -0700
[Message part 1 (text/plain, inline)]
Subject: telnet: Buffer Overrun by unchecked environment variables
Package: telnet
Version: 0.17-24
Severity: normal
Tags: patch



-- System Information:
Debian Release: 3.1
  APT prefers unstable
  APT policy: (500, 'unstable')
Architecture: i386 (i686)
Kernel: Linux 2.6.7
Locale: LANG=C, LC_CTYPE=C

Versions of packages telnet depends on:
ii  libc6                       2.3.2.ds1-15 GNU C Library: Shared libraries 
an
ii  libgcc1                     1:3.4.1-5    GCC support library
ii  libncurses5                 5.4-4        Shared libraries for terminal 
hand
ii  libstdc++5                  1:3.3.4-7    The GNU Standard C++ Library v3
ii  netbase                     4.18         Basic TCP/IP networking system

-- no debconf information
Although this should never actually happen, if you set your environment
variable HOME to an extremely large string a buffer overflow will occur upon
connecting to a server using telnet.  I was not able to overwrite 'eip'
but I have included a patch that fixes this problem.

-- 
Josh Martin
jmartin@prescientsoftware.com
[netkit-telnet-0.17.telnet.patch (text/x-diff, attachment)]

Information forwarded to debian-bugs-dist@lists.debian.org:
Bug#264846; Package telnet. Full text and rfc822 format available.

Acknowledgement sent to Robert Millan <rmh@debian.org>:
Extra info received and forwarded to list. Full text and rfc822 format available.

Message #10 received at 264846@bugs.debian.org (full text, mbox):

From: Robert Millan <rmh@debian.org>
To: Josh Martin <jmartin@columbiaservices.net>, 264846@bugs.debian.org
Cc: debian-security@lists.debian.org
Subject: Re: Bug#264846: telnet: Buffer Overrun by unchecked environment variables
Date: Wed, 11 Aug 2004 21:38:32 +0200
On Tue, Aug 10, 2004 at 10:12:06AM -0700, Josh Martin wrote:
> 
> -- no debconf information
> Although this should never actually happen, if you set your environment
> variable HOME to an extremely large string a buffer overflow will occur upon
> connecting to a server using telnet.

Urgh.. This really calls for an upload to t-p-u.

> I was not able to overwrite 'eip'
> but I have included a patch that fixes this problem.

Could you overwrite esp/ebp?  Anyway, I'm CCing the security team for
assistance on the impact.  I don't think it's release-critical since a
tainted HOME already implies there's a flaw somewhere.

> --- commands.orig.cc	2004-08-10 09:50:44.000000000 -0700
> +++ commands.cc	2004-08-10 09:51:07.000000000 -0700
> @@ -2148,7 +2148,7 @@
>      if (rcname == 0) {
>  	rcname = getenv("HOME");
>  	if (rcname)
> -	    strcpy(rcbuf, rcname);
> +	    strncpy(rcbuf, rcname, 127);
>  	else
>  	    rcbuf[0] = '\0';
>  	strcat(rcbuf, "/.telnetrc");

I don't like it.  This keeps the 127-byte hardcoded limit.  What would you
think about:

--- netkit-telnet-0.17/telnet/commands.cc~	2004-05-19 01:56:10.000000000 +0200
+++ netkit-telnet-0.17/telnet/commands.cc	2004-08-11 21:32:02.000000000 +0200
@@ -2139,22 +2139,14 @@
 }
 
 void cmdrc(const char *m1, const char *m2, const char *port) {
-    static char *rcname = 0;
-    static char rcbuf[128];
+    static char *rcname;
 
     if (skiprc) return;
 
     readrc(m1, m2, port, "/etc/telnetrc");
-    if (rcname == 0) {
-	rcname = getenv("HOME");
-	if (rcname)
-	    strcpy(rcbuf, rcname);
-	else
-	    rcbuf[0] = '\0';
-	strcat(rcbuf, "/.telnetrc");
-	rcname = rcbuf;
-    }
+    asprintf (&rcname, "%s/.telnetrc", getenv ("HOME"));
     readrc(m1, m2, port, rcname);
+    free (rcname);
 }
 
 #if defined(IP_OPTIONS) && defined(HAS_IPPROTO_IP)


Let me know if I screwed on something, we need to be extra careful with
standard packages during the freeze..

-- 
Robert Millan

(Debra and Ian) (Gnu's Not (UNiplexed Information and Computing System))/\
(kernel of *(Berkeley Software Distribution))



Information forwarded to debian-bugs-dist@lists.debian.org, Robert Millan <rmh@debian.org>:
Bug#264846; Package telnet. Full text and rfc822 format available.

Acknowledgement sent to "Bernhard R. Link" <brl@pcpool00.mathematik.uni-freiburg.de>:
Extra info received and forwarded to list. Copy sent to Robert Millan <rmh@debian.org>. Full text and rfc822 format available.

Message #15 received at 264846@bugs.debian.org (full text, mbox):

From: "Bernhard R. Link" <brl@pcpool00.mathematik.uni-freiburg.de>
To: Josh Martin <jmartin@columbiaservices.net>, 264846@bugs.debian.org
Cc: debian-security@lists.debian.org
Subject: Re: telnet: Buffer Overrun by unchecked environment variables
Date: Thu, 12 Aug 2004 10:04:52 +0200
* Josh Martin <jmartin@columbiaservices.net> [040810 10:08]:
> Although this should never actually happen, if you set your environment
> variable HOME to an extremely large string a buffer overflow will occur upon
> connecting to a server using telnet.  I was not able to overwrite 'eip'
> but I have included a patch that fixes this problem.

[some context for the patch]
void cmdrc(const char *m1, const char *m2, const char *port) {
    static char *rcname = 0;
    static char rcbuf[128];

    if (skiprc) return;

    readrc(m1, m2, port, "/etc/telnetrc");
> --- commands.orig.cc	2004-08-10 09:50:44.000000000 -0700
> +++ commands.cc	2004-08-10 09:51:07.000000000 -0700
> @@ -2148,7 +2148,7 @@
>      if (rcname == 0) {
>  	rcname = getenv("HOME");
>  	if (rcname)
> -	    strcpy(rcbuf, rcname);
> +	    strncpy(rcbuf, rcname, 127);
>  	else
>  	    rcbuf[0] = '\0';
>  	strcat(rcbuf, "/.telnetrc");


I may be utterly confused, but that patch does look quite strange.
It may make it near to impossible to introduce code, but only reduces 
the problem: strncpy will not '\0'-terminate the string, so that the
following "/.telnetrc" will be written to some random position.
and even if there was some termination, 127 chars plus 10 chars
for "/.telnetrc" is still more than the reserved 128. (thus when
having $HOME 116 to 126 chars one could even control where the
/.telnetrc letters get to).


Hochachtungsvoll,
	Bernhard R. Link



Information forwarded to debian-bugs-dist@lists.debian.org:
Bug#264846; Package telnet. Full text and rfc822 format available.

Acknowledgement sent to Robert Millan <rmh@debian.org>:
Extra info received and forwarded to list. Full text and rfc822 format available.

Message #20 received at 264846@bugs.debian.org (full text, mbox):

From: Robert Millan <rmh@debian.org>
To: Josh Martin <jmartin@columbiaservices.net>, 264846@bugs.debian.org, debian-security@lists.debian.org
Subject: Re: Bug#264846: telnet: Buffer Overrun by unchecked environment variables
Date: Thu, 12 Aug 2004 11:08:55 +0200
On Thu, Aug 12, 2004 at 10:04:52AM +0200, Bernhard R. Link wrote:
> 
> I may be utterly confused, but that patch does look quite strange.
> It may make it near to impossible to introduce code, but only reduces 
> the problem: strncpy will not '\0'-terminate the string, so that the
> following "/.telnetrc" will be written to some random position.
> and even if there was some termination, 127 chars plus 10 chars
> for "/.telnetrc" is still more than the reserved 128. (thus when
> having $HOME 116 to 126 chars one could even control where the
> /.telnetrc letters get to).

That patch is wrong.  Please direct your comments at the patch for dynamic
allocation I just sent instead.

-- 
Robert Millan

(Debra and Ian) (Gnu's Not (UNiplexed Information and Computing System))/\
(kernel of *(Berkeley Software Distribution))



Information forwarded to debian-bugs-dist@lists.debian.org:
Bug#264846; Package telnet. Full text and rfc822 format available.

Acknowledgement sent to Robert Millan <rmh@debian.org>:
Extra info received and forwarded to list. Full text and rfc822 format available.

Message #25 received at 264846@bugs.debian.org (full text, mbox):

From: Robert Millan <rmh@debian.org>
To: Wade Richards <wade@wabyn.net>
Cc: 264846@bugs.debian.org
Subject: Re: Bug#264846: telnet: Buffer Overrun by unchecked environment variables
Date: Thu, 12 Aug 2004 11:14:37 +0200
On Wed, Aug 11, 2004 at 02:09:37PM -0700, Wade Richards wrote:
> Hi,
> 
> I hope you don't mind me nit-picking...

Feedback is appreciated, of course.

> On Wed, Aug 11, 2004 at 09:38:32PM +0200, Robert Millan wrote:
> > --- netkit-telnet-0.17/telnet/commands.cc~	2004-05-19 01:56:10.000000000 +0200
> > +++ netkit-telnet-0.17/telnet/commands.cc	2004-08-11 21:32:02.000000000 +0200
> > @@ -2139,22 +2139,14 @@
> >  }
> >  
> >  void cmdrc(const char *m1, const char *m2, const char *port) {
> > -    static char *rcname = 0;
> > -    static char rcbuf[128];
> > +    static char *rcname;
> 
> Why still static?  Without knowing the rest of the program flow, I need
> to worry about whether or not this is a multi-threaded bug.  And with
> the new logic, there is no advantage to making it static.

The program is not multi-threaded, but making it static is pointless anyway.

> Also, I'd keep the initialization to NULL.  It helps to start with a
> known state.

Yep.

> >      if (skiprc) return;
> >  
> >      readrc(m1, m2, port, "/etc/telnetrc");
> > -    if (rcname == 0) {
> > -	rcname = getenv("HOME");
> > -	if (rcname)
> > -	    strcpy(rcbuf, rcname);
> > -	else
> > -	    rcbuf[0] = '\0';
> > -	strcat(rcbuf, "/.telnetrc");
> > -	rcname = rcbuf;
> > -    }
> > +    asprintf (&rcname, "%s/.telnetrc", getenv ("HOME"));
> 
> What if asprintf fails?  I think it returns 0, but you are ignoring the
> return value.  So if you "HOME" is large enough, the malloc will fail,

You mean, like, ENOMEM?  Ok, I'll add something.

Anything else?

-- 
Robert Millan

(Debra and Ian) (Gnu's Not (UNiplexed Information and Computing System))/\
(kernel of *(Berkeley Software Distribution))



Information forwarded to debian-bugs-dist@lists.debian.org, Robert Millan <rmh@debian.org>:
Bug#264846; Package telnet. Full text and rfc822 format available.

Acknowledgement sent to Wade Richards <wade@wabyn.net>:
Extra info received and forwarded to list. Copy sent to Robert Millan <rmh@debian.org>. Full text and rfc822 format available.

Message #30 received at 264846@bugs.debian.org (full text, mbox):

From: Wade Richards <wade@wabyn.net>
To: Robert Millan <rmh@debian.org>
Cc: 264846@bugs.debian.org
Subject: Re: Bug#264846: telnet: Buffer Overrun by unchecked environment variables
Date: Thu, 12 Aug 2004 02:22:33 -0700
On Thu, Aug 12, 2004 at 11:14:37AM +0200, Robert Millan wrote:
> > > +    asprintf (&rcname, "%s/.telnetrc", getenv ("HOME"));
> > 
> > What if asprintf fails?  I think it returns 0, but you are ignoring the
> > return value.  So if you "HOME" is large enough, the malloc will fail,
> 
> You mean, like, ENOMEM?  Ok, I'll add something.

Actually, I really should have read "man 3 asprintf" before replying.
If allocation fails, it will return -1, and the pointer is undefined.
So, if asprintf returns -1, you cannot trust the pointer.

    --- Wade

-- 
 /"\  . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 
 \ /   ASCII Ribbon Campaign    | Wade Richards --- wade@wabyn.net
  X   - NO HTML/RTF in e-mail   | Hagh qoHpu' neH QaghDI' ghunwI''a'pu'
 / \  - NO Word docs in e-mail  | 



Reply sent to Robert Millan <rmh@debian.org>:
You have taken responsibility. Full text and rfc822 format available.

Notification sent to Josh Martin <jmartin@columbiaservices.net>:
Bug acknowledged by developer. Full text and rfc822 format available.

Message #35 received at 264846-close@bugs.debian.org (full text, mbox):

From: Robert Millan <rmh@debian.org>
To: 264846-close@bugs.debian.org
Subject: Bug#264846: fixed in netkit-telnet 0.17-25
Date: Thu, 12 Aug 2004 22:47:05 -0400
Source: netkit-telnet
Source-Version: 0.17-25

We believe that the bug you reported is fixed in the latest version of
netkit-telnet, which is due to be installed in the Debian FTP archive:

netkit-telnet_0.17-25.diff.gz
  to pool/main/n/netkit-telnet/netkit-telnet_0.17-25.diff.gz
netkit-telnet_0.17-25.dsc
  to pool/main/n/netkit-telnet/netkit-telnet_0.17-25.dsc
telnet_0.17-25_i386.deb
  to pool/main/n/netkit-telnet/telnet_0.17-25_i386.deb
telnetd_0.17-25_i386.deb
  to pool/main/n/netkit-telnet/telnetd_0.17-25_i386.deb



A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 264846@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Robert Millan <rmh@debian.org> (supplier of updated netkit-telnet package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.7
Date: Fri, 13 Aug 2004 04:21:36 +0200
Source: netkit-telnet
Binary: telnetd telnet
Architecture: source i386
Version: 0.17-25
Distribution: unstable
Urgency: low
Maintainer: Robert Millan <rmh@debian.org>
Changed-By: Robert Millan <rmh@debian.org>
Description: 
 telnet     - The telnet client.
 telnetd    - The telnet server.
Closes: 264846
Changes: 
 netkit-telnet (0.17-25) unstable; urgency=low
 .
   * telnet/commands.cc: Fix buffer overflow when $HOME is too big.  Thanks
     Josh Martin. (Closes: #264846)
Files: 
 cb399c59dec0671a3c9999133257696b 589 net standard netkit-telnet_0.17-25.dsc
 c44a64e5307a33d364762a00ef8f16dd 25192 net standard netkit-telnet_0.17-25.diff.gz
 4310b1bf036d02db30746f1af9555b97 63812 net standard telnet_0.17-25_i386.deb
 9729bd075a4e227a31cf54c5d3a62892 40606 net optional telnetd_0.17-25_i386.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)

iD8DBQFBHCf7C19io6rUCv8RAsrbAKCFj/Z6mcQhhv+7/k689u0kBzPZrwCdF0k/
MG8QxeCpED2I0KKLAmH0h1M=
=C1XC
-----END PGP SIGNATURE-----




Send a report that this bug log contains spam.


Debian bug tracking system administrator <owner@bugs.debian.org>. Last modified: Sat Apr 19 07:59:50 2014; Machine Name: beach.debian.org

Debian Bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.