[net.bugs.4bsd] Bug in 4.2 tip

ber@enea.UUCP (Bj|rn Eriksen) (02/09/84)

When I was adding the "tip" (4.2 BSD) line locking routines to Kermit
I came across that old uucplock-bug. In the source for tip there is a 
file uucplock.c which defines

	#define NAMESIZE 15

which is used in "ulockf" as


	static char tempfile[NAMESIZE];

	if (pid < 0) {
		pid = getpid();
		sprintf(tempfile, "/usr/spool/uucp/LTMP.%d", pid);
	}

But that file name sure has more than 15 character, which caused
my new Kermit to core dump in the next calloc.
-- 
	Bjorn Eriksen	{philabs,decvax}!mcvax!enea!ber

sid@linus.UUCP (Sid Stuart) (06/30/84)

Index: 4.2 BSD tip acu.c

Description: I ran into this about a month ago right before I went
on vacation. I forget exactly what was wrong, but I think if you set
the cm variable in /etc/remote to a non-null string, it would cause
tip to dump core and die. The cm string is supposed
to be sent to the computer you are connecting to. Unfortunatly, the
write statement uses the wrong character buffer, it is obvious that
the code is incorrect, if you look at it. The statement is

	pwrite(FD, cp, size(CM);

The problem is that cp and &CM point to different buffers. So if
cp points to a null buffer and &CM points to a buffer with a string in
it... you dump your core. The fix is simple, just put &CM in place of
cp. This fix will have the side effect that the program may do
what it is supposed to at this point.

Repeat: Like I said before, I have forgotten.

Fix:

*** acu.c.orig	Sun May 20 20:13:26 1984
--- acu.c	Sun May 20 20:28:17 1984
***************
*** 34,41
  	int tried = 0;
  
  	if (!DU) {		/* regular connect message */
! 		if (CM != NOSTR)
! 			pwrite(FD, cp, size(CM));
  		return (NOSTR);
  	}
  	/*

--- 34,41 -----
  	int tried = 0;
  
  	if (!DU) {		/* regular connect message */
! 		if (CM != NOSTR) 
! 			pwrite(FD, &CM, size(CM));
  		return (NOSTR);
  	}
  	/*

terryl@tekchips.UUCP (Terry Laskodi) (07/03/84)

     Another bug in tip(actually two bugs).

     1) In uucplock.c, in the routine ulockf(), there is a static array declared
char tempfile[NAMESIZE]. Now up above at the beginning of the file, NAMESIZE
is declared to be 15. Well, back in ulockf(), an sprintf() is done into
tempfile with the string "/usr/spool/uucp/LTMP.%d", pid. Guess what folks:
that's more than 15 characters!!!! On my machine, it just so happened that
the variable Nlocks came after tempfile, so when I was trying to close the
connection, Nlocks had a bogus value in it. Oh, by the way, I looked very
briefly, and the uucp code looks like it has the same problem. The fix??
Change NAMESIZE to something more reasonable, like 32.

     2) The above bug was reasonably blatant, but just an oversight. The next
bug is actually a questionable design flaw: There is a structure array that
holds the state of certain variables of the connection, most notably the state
of the ESC character (that funny character you type, plus another magic char-
acter to exit tip). Well, this structure is declared as

	struct	{
	..........			/* other un-interesting info */
		char *v_value;		/* casted to a union later */
	}

     Now the v_value variable can hold different information, depending on
what it is used for. It can hold a real character pointer, it can hold a
boolean value, and it can hold AN ACTUAL CHARACTER(emphasis mine). Now the
problem with it holding an actual character is that the structure is
initialized with a character pointer. And as the comment says, to reference
it as an actual character, it is casted to a character in a really obnoxious
way. The whole point is that this may work fine for a VAX or PDP where the
byte address of the character pointer is the low byte of the pointer, but on
a 68000-class processor, the byte address of the pointer is the high byte of
the pointer, so when the thing is coerced into a character, you pick up the
high byte which just happens to be 0. Kind of makes it hard to exit tip.
The fix?? In tip.h, approximately line 110 (look for the typedef union named
zzhack; boy did they name it right!!), change the line

		char zz_character;

to
		int zz_character;

or whatever size pointers are on your machine(for people who still have ints
as 2 bytes on a 68000-class processor).


				Terry Laskodi
				     of
				Tektronix

honey@down.FUN (code 101) (07/05/84)

you're missing the point:  the bug in tip is its very existence.  tip
(or cu or getto or or conn or what have you) should share the conn()
function of uucico and mimic its actions in connecting to a remote.
has anyone done this?

yes, someone has done this.  honey danber has STANDALONE hooks in
conn.c and comes with a cu.c (and ct.c) that uses uucp's conn().
	peter honeyman

jdi@psuvax1.UUCP (John D. Irwin) (07/07/84)

Please stop talking about the wonderfullness of honey danber UUCP if noone
is willing to distribute it.
-- 
Spoken:	John D. Irwin
AT&T:	814-237-5068
Nets:	jdi@psuvax1.{BITNET,CSNET}
Uucp:	{akgua, allegra, cornell, pitt, purdue, ihnp4, burdvax}!psuvax1!jdi

chris@umcp-cs.UUCP (07/08/84)

[Actually, this isn't the place to discuss this, but what the heck]

Seems to me that with 4.2's ability to hand off file descriptors
across the AF_UNIX domain, there should be one single ``dialer
daemon'' which can be asked to dial a number and hand back a modem
descriptor.  Then all the modem locking, phone number logging, etc.
doesn't have to be spread out across N thousand programs.
-- 
In-Real-Life: Chris Torek, Univ of MD Comp Sci (301) 454-7690
UUCP:	{seismo,allegra,brl-bmd}!umcp-cs!chris
CSNet:	chris@umcp-cs		ARPA:	chris@maryland

per@erix.UUCP (Per Hedeland) (07/18/84)

> tip (or cu or getto or or conn or what have you) should share the conn()
> function of uucico and mimic its actions in connecting to a remote.

Considering the way conn() looks in the version of uucp we're running 
(no names), it's trouble enough that uucico uses it...