[893] in Zephyr_Bugs
fixes for buffer overflows and other bugs in zhm
daemon@ATHENA.MIT.EDU (mhpower@MIT.EDU)
Sun Sep 14 02:47:26 1997
From: <mhpower@MIT.EDU>
To: bugs@MIT.EDU, zephyr-bugs@MIT.EDU, sipb-athena-bugs@MIT.EDU
Date: Sun, 14 Sep 1997 02:47:23 EDT
It appears that zhm is vulnerable to buffer overflows, at least in
conjunction with spoofing of DNS responses, e.g.,
hm_control(notice)
...
char suggested_server[64];
...
hp = gethostbyaddr((char *) &addr, sizeof(addr), AF_INET);
if (hp != NULL) {
strcpy(suggested_server, hp->h_name);
The attached patches should fix this and related problems. Also, they
have zhm check the return value of malloc and realloc in all cases,
rather than doing so intermittently. Finally, they fix a bug whereby a
malloc failure could cause zhm to go into an infinite loop calling
hes_resolve(NULL, "sloc") every 30 seconds.
For compatibility with the existing zhm.c code, newly checked-for
malloc failures cause zhm to write the string "Out of memory.\n" to
standard output and exit with status -5. There is not necessarily any
good reason for this particular means of failure notification, and
other approaches may well be more reasonable. For similar
compatibility reasons, newly checked-for failures in the
*_notice_*_queue functions generate syslog messages containing text
beginning with the substring "Hey!".
The patches are relative to source in /mit/zephyr/src/zhm.
Finally, I have a feature request. I think there should be a supported
way to configure zhm such that it runs as a user other than root, or
(ever better) make zhm run as a specific non-root user by default. One
unsupported approach would be to modify the CFLAGS line in the
Makefile to include -D_PATH_VARRUN=\"/var/athena/daemon/\", create a
directory /var/athena/daemon owned by user daemon, and modify
/etc/init.d/athena to run "su daemon -c /etc/athena/zhm" rather than
"/etc/athena/zhm" and refer to "/var/athena/daemon/zhm.pid" rather
than "/var/athena/zhm.pid".
Matt
*** zhm.c.old Sun Dec 15 18:36:31 1996
--- zhm.c Sat Sep 13 23:22:57 1997
***************
*** 122,125 ****
printf("Unknown server name: %s\n", argv[optind-1]);
! } else
! strcpy(prim_serv, hp->h_name);
--- 122,127 ----
printf("Unknown server name: %s\n", argv[optind-1]);
! } else {
! strncpy(prim_serv, hp->h_name, sizeof(prim_serv));
! prim_serv[sizeof(prim_serv) - 1] = '\0';
! }
***************
*** 127,128 ****
--- 129,134 ----
serv_list = (char **) malloc((argc - optind + 2) * sizeof(char *));
+ if (serv_list == NULL) {
+ printf("Out of memory.\n");
+ exit(-5);
+ }
serv_list[numserv++] = prim_serv;
***************
*** 269,272 ****
printf("Hesiod error getting primary server info.\n");
! } else
! strcpy(prim_serv, c+1);
break;
--- 275,280 ----
printf("Hesiod error getting primary server info.\n");
! } else {
! strncpy(prim_serv, c+1, sizeof(prim_serv));
! prim_serv[sizeof(prim_serv) - 1] = '\0';
! }
break;
***************
*** 294,295 ****
--- 302,307 ----
strcpy(zcluster, "zephyr");
+ else {
+ printf("Out of memory.\n");
+ exit(-5);
+ }
}
***************
*** 301,302 ****
--- 313,318 ----
clust_info = (char **) malloc(2 * sizeof(char *));
+ if (clust_info == NULL) {
+ printf("Out of memory.\n");
+ exit(-5);
+ }
if (prim_serv[0])
***************
*** 309,310 ****
--- 325,330 ----
(numserv+2) * sizeof(char *));
+ if (clust_info == NULL) {
+ printf("Out of memory.\n");
+ exit(-5);
+ }
clust_info[numserv++] = strsave(serv_list[i]);
***************
*** 318,320 ****
srandom(time(NULL));
! strcpy(prim_serv, serv_list[random() % numserv]);
}
--- 338,341 ----
srandom(time(NULL));
! strncpy(prim_serv, serv_list[random() % numserv], sizeof(prim_serv));
! prim_serv[sizeof(prim_serv) - 1] = '\0';
}
***************
*** 343,346 ****
! if (*prim_serv == '\0')
! strcpy(prim_serv, *serv_list);
--- 364,369 ----
! if (*prim_serv == '\0') {
! strncpy(prim_serv, *serv_list, sizeof(prim_serv));
! prim_serv[sizeof(prim_serv) - 1] = '\0';
! }
***************
*** 397,400 ****
DPR2("Server = %s\n", prim_serv);
! strcpy(cur_serv, prim_serv);
! memcpy(&serv_sin.sin_addr, hp->h_addr, hp->h_length);
}
--- 420,424 ----
DPR2("Server = %s\n", prim_serv);
! strncpy(cur_serv, prim_serv, sizeof(cur_serv));
! cur_serv[sizeof(cur_serv) - 1] = '\0';
! memcpy(&serv_sin.sin_addr, hp->h_addr, 4);
}
***************
*** 477,478 ****
--- 501,506 ----
list[0] = (char *) malloc(MAXHOSTNAMELEN);
+ if (list[0] == NULL) {
+ printf("Out of memory.\n");
+ exit(-5);
+ }
strcpy(list[0], cur_serv);
***************
*** 479,480 ****
--- 507,512 ----
list[1] = (char *) malloc(64);
+ if (list[1] == NULL) {
+ printf("Out of memory.\n");
+ exit(-5);
+ }
sprintf(list[1], "%d", queue_len());
***************
*** 481,482 ****
--- 513,518 ----
list[2] = (char *) malloc(64);
+ if (list[2] == NULL) {
+ printf("Out of memory.\n");
+ exit(-5);
+ }
sprintf(list[2], "%d", nclt);
***************
*** 483,484 ****
--- 519,524 ----
list[3] = (char *) malloc(64);
+ if (list[3] == NULL) {
+ printf("Out of memory.\n");
+ exit(-5);
+ }
sprintf(list[3], "%d", nserv);
***************
*** 485,486 ****
--- 525,530 ----
list[4] = (char *) malloc(64);
+ if (list[4] == NULL) {
+ printf("Out of memory.\n");
+ exit(-5);
+ }
sprintf(list[4], "%d", nservchang);
***************
*** 487,490 ****
list[5] = (char *) malloc(64);
! strcpy(list[5], rcsid_hm_c);
list[6] = (char *) malloc(64);
if (no_server)
--- 531,544 ----
list[5] = (char *) malloc(64);
! if (list[5] == NULL) {
! printf("Out of memory.\n");
! exit(-5);
! }
! strncpy(list[5], rcsid_hm_c, 64);
! list[5][63] = '\0';
!
list[6] = (char *) malloc(64);
+ if (list[6] == NULL) {
+ printf("Out of memory.\n");
+ exit(-5);
+ }
if (no_server)
***************
*** 494,495 ****
--- 548,553 ----
list[7] = (char *) malloc(64);
+ if (list[7] == NULL) {
+ printf("Out of memory.\n");
+ exit(-5);
+ }
sprintf(list[7], "%ld", time((time_t *)0) - starttime);
***************
*** 502,503 ****
--- 560,565 ----
list[8] = (char *)malloc(64);
+ if (list[8] == NULL) {
+ printf("Out of memory.\n");
+ exit(-5);
+ }
sprintf(list[8], "%ld", size);
***************
*** 504,506 ****
list[9] = (char *)malloc(32);
! strcpy(list[9], MACHINE_TYPE);
--- 566,573 ----
list[9] = (char *)malloc(32);
! if (list[9] == NULL) {
! printf("Out of memory.\n");
! exit(-5);
! }
! strncpy(list[9], MACHINE_TYPE, 32);
! list[9][31] = '\0';
*** zhm_client.c.old Wed Apr 3 18:01:34 1996
--- zhm_client.c Sun Sep 14 01:56:22 1997
***************
*** 78,80 ****
}
! add_notice_to_queue(notice, packet, &gsin, pak_len);
}
--- 78,81 ----
}
! if (add_notice_to_queue(notice, packet, &gsin, pak_len) != ZERR_NONE)
! syslog(LOG_INFO, "Hey! Insufficient memory to add notice to queue!");
}
*** zhm_server.c.old Wed Apr 3 18:01:35 1996
--- zhm_server.c Sun Sep 14 02:31:32 1997
***************
*** 115,117 ****
DPR2 ("Server = %s\n", sugg_serv);
! (void)strcpy(cur_serv, sugg_serv);
if (hmdebug)
--- 115,118 ----
DPR2 ("Server = %s\n", sugg_serv);
! (void)strncpy(cur_serv, sugg_serv, MAXHOSTNAMELEN);
! cur_serv[MAXHOSTNAMELEN - 1] = '\0';
if (hmdebug)
***************
*** 127,129 ****
DPR2 ("Server = %s\n", prim_serv);
! (void)strcpy(cur_serv, prim_serv);
done = 1;
--- 128,131 ----
DPR2 ("Server = %s\n", prim_serv);
! (void)strncpy(cur_serv, prim_serv, MAXHOSTNAMELEN);
! cur_serv[MAXHOSTNAMELEN - 1] = '\0';
done = 1;
***************
*** 137,139 ****
DPR2 ("Server = %s\n", *serv_list);
! (void)strcpy(cur_serv, *serv_list);
done = 1;
--- 139,142 ----
DPR2 ("Server = %s\n", *serv_list);
! (void)strncpy(cur_serv, *serv_list, MAXHOSTNAMELEN);
! cur_serv[MAXHOSTNAMELEN - 1] = '\0';
done = 1;
***************
*** 155,157 ****
DPR2 ("Server = %s\n", new_serv);
! (void)strcpy(cur_serv, new_serv);
done = 1;
--- 158,161 ----
DPR2 ("Server = %s\n", new_serv);
! (void)strncpy(cur_serv, new_serv, MAXHOSTNAMELEN);
! cur_serv[MAXHOSTNAMELEN - 1] = '\0';
done = 1;
***************
*** 163,165 ****
}
! (void) memcpy((char *)&serv_sin.sin_addr, hp->h_addr, hp->h_length);
nservchang++;
--- 167,169 ----
}
! (void) memcpy((char *)&serv_sin.sin_addr, hp->h_addr, 4);
nservchang++;
***************
*** 201,203 ****
struct hostent *hp;
! char suggested_server[64];
unsigned long addr;
--- 205,207 ----
struct hostent *hp;
! char suggested_server[MAXHOSTNAMELEN];
unsigned long addr;
***************
*** 210,212 ****
if (hp != NULL) {
! strcpy(suggested_server, hp->h_name);
new_server(suggested_server);
--- 214,217 ----
if (hp != NULL) {
! strncpy(suggested_server, hp->h_name, sizeof(suggested_server));
! suggested_server[sizeof(suggested_server) - 1] = '\0';
new_server(suggested_server);
*** queue.c.old Sun Mar 3 21:44:34 1996
--- queue.c Sun Sep 14 01:11:47 1997
***************
*** 70,71 ****
--- 70,73 ----
entry = (Queue *) malloc(sizeof(Queue));
+ if (entry == NULL)
+ return(ZERR_NONOTICE);
entry->retries = 0;
***************
*** 72,73 ****
--- 74,79 ----
entry->packet = (char *) malloc(Z_MAXPKTLEN);
+ if (entry->packet == NULL) {
+ free(entry);
+ return(ZERR_NONOTICE);
+ }
memcpy(entry->packet, packet, Z_MAXPKTLEN);
