Bug 13535

Summary: A scurity bug of finger
Product: Base System Reporter: Yoshihiro Koya <Yoshihiro.Koya>
Component: binAssignee: freebsd-bugs (Nobody) <bugs>
Status: Closed FIXED    
Severity: Affects Only Me CC: koya
Priority: Normal    
Version: 2.2.8-RELEASE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff none

Description Bill Fumerola 1999-09-02 04:25:04 UTC
On Thu, 2 Sep 1999, Yoshihiro Koya wrote:

> By using chpass programm, change the gecos field of a some user as
> follows:
> 
>   >#Changing user database information for someuser
>   >Shell: /bin/csh
>   >Full Name: xxxxxxxxxxxxxxxxx... ( put here 1500 times x, for example )
>   >Location:
>   >Office Phone:
>   >Home Phone:
>   >Other information:

This makes NIS ignore you as a user because it crashes mknetid. Neat.

-- 
- bill fumerola - billf@chc-chimes.com - BF1560 - computer horizons corp -
- ph:(800) 252-2421 - bfumerol@computerhorizons.com - billf@FreeBSD.org  -
Comment 1 Yoshihiro Koya 1999-09-02 04:50:01 UTC
When the finger program prints user information of a system, it refers
the gecos field of passwd database.  The length of the gecos field may
be allowed up to 2048 bytes. On the other hand, almost all buffer of
the finger program has only 1024 bytes.

The "amphersand problem" of finger in the current CVS repository has
been already fixed.  However, even it has the problem described above.

Fix: Apply the patch below:

----------------------------------------------------------------------


------------------------------------------------------------------------78bbDGWYai3CLUj8L5YEWdjv6vurGA9yBdeWzp5qjgd2NjuC
Content-Type: text/plain; name="file.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="file.diff"

diff -c /usr/src/usr.bin/finger/extern.h /home/koya/labo/finger/extern.h
*** /usr/src/usr.bin/finger/extern.h	Thu Jul  3 16:12:37 1997
--- /home/koya/labo/finger/extern.h	Thu Sep  2 12:15:18 1999
***************
*** 33,39 ****
   *	@(#)extern.h	8.1 (Berkeley) 6/6/93
   */
  
! extern char tbuf[1024];			/* Temp buffer for anybody. */
  extern int entries;			/* Number of people. */
  extern DB *db;				/* Database. */
  
--- 33,39 ----
   *	@(#)extern.h	8.1 (Berkeley) 6/6/93
   */
  
! extern char tbuf[LINE_MAX];		/* Temp buffer for anybody. */
  extern int entries;			/* Number of people. */
  extern DB *db;				/* Database. */
  
diff -c /usr/src/usr.bin/finger/finger.c /home/koya/labo/finger/finger.c
*** /usr/src/usr.bin/finger/finger.c	Sun Mar  8 18:08:00 1998
--- /home/koya/labo/finger/finger.c	Thu Sep  2 12:15:40 1999
***************
*** 94,100 ****
  DB *db;
  time_t now;
  int entries, lflag, mflag, pplan, sflag, oflag, Tflag;
! char tbuf[1024];
  
  static void loginlist __P((void));
  static void usage __P((void));
--- 94,100 ----
  DB *db;
  time_t now;
  int entries, lflag, mflag, pplan, sflag, oflag, Tflag;
! char tbuf[LINE_MAX];
  
  static void loginlist __P((void));
  static void usage __P((void));
How-To-Repeat: 
By using chpass programm, change the gecos field of a some user as
follows:

  >#Changing user database information for someuser
  >Shell: /bin/csh
  >Full Name: xxxxxxxxxxxxxxxxx... ( put here 1500 times x, for example )
  >Location:
  >Office Phone:
  >Home Phone:
  >Other information:

and, type

  host% finger root

or 

  host% finger root@localhost 

if fingerd is available on your system.  Then, the finger will catch
signal 10 or 11.

In the latter, it's also possible to execute any commands by using
this under the privilege of "nobody" (It's dangerous under the system
running squid).
Comment 2 Sheldon Hearn 1999-09-02 11:49:31 UTC
On Thu, 02 Sep 1999 12:50:11 +0900, Yoshihiro Koya wrote:

> Apply the patch below:

What is your patch supposed to do? It only doubles the size of a buffer
which we _always_ seem to do bounded manipulation on. Can you spot
anything in the finger source that fiddles with tput without checking
its size and termination? _That_ would make for a proper fix.

Ciao,
Sheldon.
Comment 3 Yoshihiro Koya 1999-09-02 12:13:13 UTC
From: Sheldon Hearn <sheldonh@uunet.co.za>
Subject: Re: bin/13535: Finger bug: possible stack overflow 
Date: Thu, 02 Sep 1999 12:49:31 +0200

> On Thu, 02 Sep 1999 12:50:11 +0900, Yoshihiro Koya wrote:
> 
> > Apply the patch below:
> 
> What is your patch supposed to do? It only doubles the size of a buffer
> which we _always_ seem to do bounded manipulation on. Can you spot
> anything in the finger source that fiddles with tput without checking
> its size and termination? _That_ would make for a proper fix.
> 

I checked the source code of chpass ( /usr/src/usr.bin/chpass/edit.c ).
I found there the chpass program assumes that the length of gecos is
less than ABOUT 2048 bytes. This is the reason why I put LINE_MAX there.

About the manupulation on bound, you are right. I only paid my
attention to the size of buffer.  As you said, the current version of
the source code in the CVS repository has no problem. Sorry for my
misunderstanding.

koya
Comment 4 Sheldon Hearn 1999-09-02 12:48:13 UTC
On Thu, 02 Sep 1999 20:13:13 +0900, Yoshihiro Koya wrote:

> About the manupulation on bound, you are right. I only paid my
> attention to the size of buffer.  As you said, the current version of
> the source code in the CVS repository has no problem. Sorry for my
> misunderstanding.

No problem. :-)

So are you happy with me closing your PR, or is there a real problem
that needs to be addressed? (I haven't looked into the problem, I just
scanned the finger source for its use of the buffer).

Ciao,
Sheldon.
Comment 5 Yoshihiro Koya 1999-09-02 14:02:20 UTC
From: Sheldon Hearn <sheldonh@uunet.co.za>
Subject: Re: bin/13535: Finger bug: possible stack overflow 
Date: Thu, 02 Sep 1999 13:48:13 +0200

> So are you happy with me closing your PR, or is there a real problem
> that needs to be addressed? (I haven't looked into the problem, I just
> scanned the finger source for its use of the buffer).

There might be no probelm, I think.  I quite agree. Thank you very
much for your kind messages. 

koya
Comment 6 Sheldon Hearn freebsd_committer freebsd_triage 1999-09-02 19:26:43 UTC
State Changed
From-To: open->closed

Committed to the RELENG_2_2 branch as rev 1.3.6.3 of util.c, with  
imp's permission.