[496] in testers
rt 6.4A: zwrite
daemon@ATHENA.MIT.EDU (daemon@ATHENA.MIT.EDU)
Fri Dec 15 00:57:49 1989
Date: Fri, 15 Dec 89 00:56:53 -0500
From: Jonathan I. Kamens <jik@PIT-MANAGER.MIT.EDU>
To: testers@ATHENA.MIT.EDU, bug-zephyr@ATHENA.MIT.EDU
System name: pit-manager
Type and version: RTPC-ROMPC 6.4A
Display type: apa16
megapel
Zwrite has several somewhat serious bugs, which I found while
digging around to fix the first one:
1. It assumes a maximum of 100 recipients specified on the command
line, and makes no effort whatsoever to deal gracefully with more
than that amount (i.e. it tries to write into the 101st member of a
100-element array and dies with a segmentation violation, bus
error, or some other equally unexplanatory bug). To see an example
of this, put your username into a file called foo 101 times and
type "zwrite -n -d `cat foo` -m frep".
2. It doesn't bother to check the return value of malloc() or
realloc() in most cases, and therefore (once again) will die wtih a
segmentation violation or bus error rather than deal responsibly
with a malloc() or realloc() failure. To see an example of this,
type "limit datasize 1000k" and then type "zwrite $user < /vmunix".
3. It does not correctly null-terminate the user's signature when
reading that signature out of the /etc/passwd file. I can't give
you an example of how to duplicate this, because I detected it
under saber and I'm not sure it can be detected in most cases
elsewhere, because of how malloc() allocates blocks of memory.
4. It tries to write one past the end of a malloc()'d string when
reading the message from the terminal. This error, too, probably
cannot be detected outside of saber or another debugger, but saber
complains mighty loud about it.
I have fixed these bugs in the following ways:
1. It now assumes no maximum number of recipients, and allocates the
space to hold the recipient list in chunks of 25 char *'s. The
increment of 25 seems reasonable to me, but if you want another
increment size it can be changed by changing the macro RECIP_HUNK
at the top of the file.
2. All calls to malloc() or realloc() now call Malloc() or Realloc(),
which are functions in zwrite.c which call com_err() and exit if
the real malloc() or realloc() fails.
3. The user's signature is now correctly null-terminated.
4. The block of code that gets the message from the terminal is
modified slightly to correctly deal with realloc()'ing space and
with the NULL at the end of the message.
Patch is below.
jik
*************************
*** /paris/source/4.3/athena.lib/zephyr/clients/zwrite/zwrite.c Thu Nov 9 05:45:11 1989
--- zwrite.c Fri Dec 15 00:38:58 1989
***************
*** 18,23 ****
--- 18,24 ----
#include <netdb.h>
#include <pwd.h>
#include <ctype.h>
+ #include <errno.h>
#ifndef lint
static char rcsid_zwrite_c[] = "$Id: zwrite.c,v 1.29 89/10/26 17:06:29 jtkohl Exp $";
***************
*** 28,37 ****
#define URGENT_INSTANCE "URGENT"
#define FILSRV_CLASS "FILSRV"
! #define MAXRECIPS 100
int nrecips, msgarg, verbose, quiet, nodot;
! char *whoami, *inst, *class, *recips[MAXRECIPS];
int (*auth)();
void un_tabify();
--- 29,38 ----
#define URGENT_INSTANCE "URGENT"
#define FILSRV_CLASS "FILSRV"
! #define RECIP_HUNK 25
int nrecips, msgarg, verbose, quiet, nodot;
! char *whoami, *inst, *class, **recips = (char **) NULL;
int (*auth)();
void un_tabify();
***************
*** 38,43 ****
--- 39,46 ----
extern char *malloc(), *realloc();
char *fix_filsrv_inst();
+ char *Malloc(), *Realloc();
+
main(argc, argv)
int argc;
char *argv[];
***************
*** 85,91 ****
for (;arg<argc&&!msgarg;arg++) {
if (*argv[arg] != '-') {
! recips[nrecips++] = argv[arg];
continue;
}
if (strlen(argv[arg]) > 2)
--- 88,97 ----
for (;arg<argc&&!msgarg;arg++) {
if (*argv[arg] != '-') {
! if (add_recip(&nrecips, &recips, argv[arg])) {
! fprintf(stderr, "Too many recipients.");
! usage(whoami);
! }
continue;
}
if (strlen(argv[arg]) > 2)
***************
*** 184,189 ****
--- 190,196 ----
} else
*cp++ = *cp2;
}
+ *cp = '\0';
signature = sigbfr;
}
}
***************
*** 224,236 ****
message = NULL;
msgsize = 0;
if (signature) {
! message = malloc((unsigned)(strlen(signature)+2));
(void) strcpy(message, signature);
msgsize = strlen(message);
message[msgsize++] = '\n';
message[msgsize++] = '\0';
} else {
! message = malloc(1);
message[msgsize++] = '\0';
}
--- 231,243 ----
message = NULL;
msgsize = 0;
if (signature) {
! message = Malloc((unsigned)(strlen(signature)+2));
(void) strcpy(message, signature);
msgsize = strlen(message);
message[msgsize++] = '\n';
message[msgsize++] = '\0';
} else {
! message = Malloc(1);
message[msgsize++] = '\0';
}
***************
*** 239,245 ****
for (arg=msgarg;arg<argc;arg++)
size += (strlen(argv[arg]) + 1);
size++; /* for the newline */
! message = realloc(message, (unsigned) size);
for (arg=msgarg;arg<argc;arg++) {
(void) strcpy(message+msgsize, argv[arg]);
msgsize += strlen(argv[arg]);
--- 246,252 ----
for (arg=msgarg;arg<argc;arg++)
size += (strlen(argv[arg]) + 1);
size++; /* for the newline */
! message = Realloc(message, (unsigned) size);
for (arg=msgarg;arg<argc;arg++) {
(void) strcpy(message+msgsize, argv[arg]);
msgsize += strlen(argv[arg]);
***************
*** 253,258 ****
--- 260,274 ----
msgsize += 2;
} else {
if (isatty(0)) {
+ /* Add a NULL at the end of the message, but do not */
+ /* increment msgsize yet -- that's done at the end. The */
+ /* purpose of this extra NULL is in case there is no input */
+ /* from the terminal -- in that case, the Realloc below */
+ /* will never be executed, so the loop below will never */
+ /* put a NULL in at the end of the message, so we do it */
+ /* here first, just to be safe. */
+ message = Realloc(message, (unsigned)(msgsize + 1));
+ message[msgsize] = '\0';
for (;;) {
if (!fgets(bfr, sizeof bfr, stdin))
break;
***************
*** 259,270 ****
if (!nodot && bfr[0] == '.' &&
(bfr[1] == '\n' || bfr[1] == '\0'))
break;
! message = realloc(message, (unsigned)(msgsize+strlen(bfr)));
(void) strcpy(message+msgsize, bfr);
msgsize += strlen(bfr);
}
! message = realloc(message, (unsigned)(msgsize+1));
! message[msgsize++] = '\0';
}
else { /* Use read so you can send binary messages... */
while (nchars = read(fileno(stdin), bfr, sizeof bfr)) {
--- 275,290 ----
if (!nodot && bfr[0] == '.' &&
(bfr[1] == '\n' || bfr[1] == '\0'))
break;
! /* The +1 is for the NULL which strcpy wants to be */
! /* able to put at the end of the string it copies. */
! message = Realloc(message, (unsigned)(msgsize+strlen(bfr)+1));
(void) strcpy(message+msgsize, bfr);
msgsize += strlen(bfr);
}
! /* Now that the message has been completely read, */
! /* increment msgsize by 1 so that the NULL at the end of */
! /* it will be part of the message that is sent. */
! msgsize += 1;
}
else { /* Use read so you can send binary messages... */
while (nchars = read(fileno(stdin), bfr, sizeof bfr)) {
***************
*** 272,283 ****
fprintf(stderr, "Read error from stdin! Can't continue!\n");
exit(1);
}
! message = realloc(message, (unsigned)(msgsize+nchars));
bcopy(bfr, message+msgsize, nchars);
msgsize += nchars;
}
/* end of msg */
! message = realloc(message, (unsigned)(msgsize+1));
message[msgsize++] = '\0'; /* null-terminate */
}
}
--- 292,303 ----
fprintf(stderr, "Read error from stdin! Can't continue!\n");
exit(1);
}
! message = Realloc(message, (unsigned)(msgsize+nchars));
bcopy(bfr, message+msgsize, nchars);
msgsize += nchars;
}
/* end of msg */
! message = Realloc(message, (unsigned)(msgsize+1));
message[msgsize++] = '\0'; /* null-terminate */
}
}
***************
*** 462,470 ****
every tab into TABSTOP spaces */
/* only add (TABSTOP-1)x because we re-use the cell holding the
tab itself */
! cp = malloc((unsigned)(*sizep + (i * (TABSTOP-1))));
! if (!cp) /* XXX */
! return; /* punt expanding if memory fails */
cp3 = cp;
/* Copy buffer, converting tabs to spaces as we go */
for (cp2 = *bufp, column = 1, size = *sizep; size; cp2++, size--) {
--- 482,488 ----
every tab into TABSTOP spaces */
/* only add (TABSTOP-1)x because we re-use the cell holding the
tab itself */
! cp = Malloc((unsigned)(*sizep + (i * (TABSTOP-1))));
cp3 = cp;
/* Copy buffer, converting tabs to spaces as we go */
for (cp2 = *bufp, column = 1, size = *sizep; size; cp2++, size--) {
***************
*** 497,499 ****
--- 515,581 ----
*bufp = cp3;
return;
}
+
+
+ add_recip(nrecips_ptr, recips_ptr, recip)
+ int *nrecips_ptr;
+ char ***recips_ptr;
+ char *recip;
+ {
+ int nrecips;
+ char **recips;
+
+ nrecips = *nrecips_ptr;
+ recips = *recips_ptr;
+
+ if (nrecips++ % RECIP_HUNK == 0) {
+ int newsize = nrecips + RECIP_HUNK - 1;
+
+ if (recips)
+ recips = (char **)
+ Realloc(recips, (unsigned) (sizeof(char *) * newsize));
+ else
+ recips = (char **)
+ Malloc((unsigned) (sizeof(char *) * newsize));
+ }
+
+ recips[nrecips - 1] = recip;
+
+ *nrecips_ptr = nrecips;
+ *recips_ptr = recips;
+
+ return (0);
+ }
+
+
+ char *Malloc(size)
+ unsigned size;
+ {
+ char *ptr;
+
+ ptr = malloc(size);
+
+ if (! ptr) {
+ com_err(whoami, errno, "");
+ exit(1);
+ }
+
+ return (ptr);
+ }
+
+
+ char *Realloc(ptr, size)
+ char *ptr;
+ unsigned size;
+ {
+ ptr = realloc(ptr, size);
+
+ if (! ptr) {
+ com_err(whoami, errno, "");
+ exit(1);
+ }
+
+ return (ptr);
+ }
+
+
