[comp.bugs.4bsd] Bug in finger.c

jfh@greenber.austin.ibm.com (John F Haugh II) (05/25/91)

In .../src/ucb/finger.c, there is a code fragment down around line 1074
(version 5.8 dated 3/13/86) that reads

	if (isprint(c) || isspace(c))
		putchar(c);
	else
		putchar(c ^ 100);

Now what are they doing XOR'ing that poor character against 100 =decimal=?
-- 
John F. Haugh II      |      I've Been Moved     |    MaBellNet: (512) 838-4340
SneakerNet: 809/1D064 |          AGAIN !         |      VNET: LCCB386 at AUSVMQ
BangNet: ..!cs.utexas.edu!ibmchs!auschs!snowball.austin.ibm.com!jfh (e-i-e-i-o)

ag@amix.commodore.com (Keith Gabryelski) (05/26/91)

Crossposted to comp.bugs.sys5 becausethe bug is in SVR4, also.

In article <7961@awdprime.UUCP> jfh@greenber.austin.ibm.com (John F Haugh II)
writes:
>In .../src/ucb/finger.c, there is a code fragment down around line 1074
>(version 5.8 dated 3/13/86) that reads
>
>	if (isprint(c) || isspace(c))
>		putchar(c);
>	else
>		putchar(c ^ 100);
>
>Now what are they doing XOR'ing that poor character against 100 =decimal=?

I noticed this happens in three places in the code.  Even if the
code used ``c ^ 0100'' a user could still send a CSI (0x9b).

Pax, Keith

Ps, My diffs: (for a SVR4 machine so line numbers may be bogus)

*** finger.c-	Sun May 26 10:49:02 1991
--- finger.c	Sun May 26 10:50:28 1991
***************
*** 491,497 ****
  						if (isprint(c) || isspace(c))
  							putchar(c);
  						else
! 							putchar(c ^ 100);
  					}
  					fclose(fp);
  					putchar('\n');
--- 491,497 ----
  						if (isprint(c) || isspace(c))
  							putchar(c);
  						else
! 						    break;
  					}
  					fclose(fp);
  					putchar('\n');
***************
*** 511,517 ****
  						if (isprint(c) || isspace(c))
  							putchar(c);
  						else
! 							putchar(c ^ 100);
  					fclose(fp);
  				}
  				free(s);
--- 511,517 ----
  						if (isprint(c) || isspace(c))
  							putchar(c);
  						else
! 						    break;
  					fclose(fp);
  				}
  				free(s);
***************
*** 1022,1028 ****
  		if (isprint(c) || isspace(c))
  			putchar(c);
  		else
! 			putchar(c ^ 100);
  	}
  	if (lastc != '\n')
  		putchar('\n');
--- 1022,1028 ----
  		if (isprint(c) || isspace(c))
  			putchar(c);
  		else
! 		    break;
  	}
  	if (lastc != '\n')
  		putchar('\n');
-- 
Keith Gabryelski                                 Advanced Products Group
ag@amix.commodore.com                                 ...!cbmvax!amix!ag

jfh@greenber.austin.ibm.com (John F Haugh II) (05/28/91)

In article <2373@amix.commodore.com> ag@amix.commodore.com (Keith Gabryelski) writes:
>I noticed this happens in three places in the code.  Even if the
>code used ``c ^ 0100'' a user could still send a CSI (0x9b).

[ ... ]
>! 							putchar(c ^ 100);

>! 						    break;

I had something a tad less drastic in mind - perhaps

	printf ("\\%0o", c);

Someone may actually care about what those characters were, or like to
know that something was there at all.
-- 
John F. Haugh II      |      I've Been Moved     |    MaBellNet: (512) 838-4340
SneakerNet: 809/1D064 |          AGAIN !         |      VNET: LCCB386 at AUSVMQ
BangNet: ..!cs.utexas.edu!ibmchs!auschs!snowball.austin.ibm.com!jfh (e-i-e-i-o)

john@loverso.leom.ma.us (John Robert LoVerso) (05/30/91)

This is fixed in the 4.3BSD-reno "finger".  FTP'able from uunet.uu.net,
in /bsd-sources/usr.bin/finger.  This source is freely redistributable,
as opposed to the finger from "tahoe".

John

ag@amix.commodore.com (Keith Gabryelski) (05/31/91)

In article <7986@awdprime.UUCP> jfh@greenber.austin.ibm.com (John F Haugh II)
writes:
>I had something a tad less drastic in mind - perhaps
>
>	printf ("\\%0o", c);
>
>Someone may actually care about what those characters were, or like
>to know that something was there at all.

Neither case is very interesting.  If someone is that interested in
such things then they should dump the file directly using od or the
like.

The Plan and Project files are restricted in that they must be one
line and not contain characters that may cause terminals to do weird
things.  I see no reason to add extra code to finger to massage a
badly (possibly maliciously) formatted file.

Others may disagree and I have heard from a number of them.

Pax, Keith
-- 
Keith Gabryelski                                 Advanced Products Group
ag@amix.commodore.com                                 ...!cbmvax!amix!ag

jik@cats.ucsc.edu (Jonathan I Kamens) (06/01/91)

In article <2405@amix.commodore.com>, ag@amix.commodore.com (Keith Gabryelski) writes:
|> The Plan and Project files are restricted in that they must be one
|> line...

The .project file is supposed to be one line long; the .plan file, however, is
allowed to be, and usually is, more than one line long.  

imp@solbourne.com (Warner Losh) (06/01/91)

In article <2405@amix.commodore.com> ag@amix.commodore.com (Keith Gabryelski) writes:
>The Plan and Project files are restricted in that they must be one
>line and not contain characters that may cause terminals to do weird
>things.

With the exception of the one line limit, I agree with you here.
What's wrong with having a two or three line plan?  I have seen
teachers use it to list phone numbers/prefered accounts for e-mail,
etc.  I have even seen some people use it to indicate if they are
actually logged in or not.  However, there should be some protection
against unprintable stuff in the plan/project files.

Warner
-- 
Warner Losh		imp@Solbourne.COM
Free to a good home: 10,000 Miller Moths.  Must promise not to breed them.