hansen@pegasus.UUCP (Tony L. Hansen) (04/27/85)
I've found a few bugs in the termcap library that still exist in
4.2BSD.
1: tgetent() cannot be called more than once in a program (hopcount doesn't
get rezeroed).
2: an entry with ":tc=foo" but no trailing colon will cause coredumps.
3. buffer overflows can still occur, causing coredumps.
4. in tc=foo, foo can only be 16 chars long. this is not checked.
Apply patch at will.
Tony Hansen
pegasus!hansen
*** /tmp/termcap.c Fri Apr 26 16:41:17 1985
--- /tmp/ntermcap.c Fri Apr 26 16:57:40 1985
***************
*** 2,8
static char sccsid[] = "@(#)termcap.c 4.1 (Berkeley) 6/27/83";
#endif
! #define BUFSIZ 1024
#define MAXHOP 32 /* max number of tc= indirections */
#define E_TERMCAP "/etc/termcap"
--- 2,8 -----
static char sccsid[] = "@(#)termcap.c 4.1 (Berkeley) 6/27/83";
#endif
! #define TBUFSIZE 2048
#define MAXHOP 32 /* max number of tc= indirections */
#define E_TERMCAP "/etc/termcap"
***************
*** 36,41
tgetent(bp, name)
char *bp, *name;
{
register char *cp;
register int c;
register int i = 0, cnt = 0;
--- 36,59 -----
tgetent(bp, name)
char *bp, *name;
{
+ /* Tony Hansen */
+ int ret;
+ hopcount = 0;
+ ret = _tgetent(bp, name);
+ /*
+ There is some sort of bug in the check way down below to prevent
+ buffer overflow. I really don't want to track it down, so I
+ upped the standard buffer size and check here to see if the created
+ buffer is larger than the old buffer size.
+ */
+ if (strlen(bp) >= 1024)
+ write(2,"Termcap entry too long\n", 23);
+ return ret;
+ }
+
+ static _tgetent(bp, name)
+ char *bp, *name;
+ {
register char *cp;
register int c;
register int i = 0, cnt = 0;
***************
*** 39,45
register char *cp;
register int c;
register int i = 0, cnt = 0;
! char ibuf[BUFSIZ];
char *cp2;
int tf;
--- 57,63 -----
register char *cp;
register int c;
register int i = 0, cnt = 0;
! char ibuf[TBUFSIZ];
char *cp2;
int tf;
***************
*** 77,83
cp = bp;
for (;;) {
if (i == cnt) {
! cnt = read(tf, ibuf, BUFSIZ);
if (cnt <= 0) {
close(tf);
return (0);
--- 95,101 -----
cp = bp;
for (;;) {
if (i == cnt) {
! cnt = read(tf, ibuf, TBUFSIZE);
if (cnt <= 0) {
close(tf);
return (0);
***************
*** 92,98
}
break;
}
! if (cp >= bp+BUFSIZ) {
write(2,"Termcap entry too long\n", 23);
break;
} else
--- 110,116 -----
}
break;
}
! if (cp >= bp+TBUFSIZE) {
write(2,"Termcap entry too long\n", 23);
break;
} else
***************
*** 120,125
tnchktc()
{
register char *p, *q;
char tcname[16]; /* name of similar terminal */
char tcbuf[BUFSIZ];
char *holdtbuf = tbuf;
--- 138,145 -----
tnchktc()
{
register char *p, *q;
+ #define TERMNAMESIZE 16
+ char tcname[TERMNAMESIZE]; /* name of similar terminal */
char tcname[16]; /* name of similar terminal */
char tcbuf[TBUFSIZE];
char *holdtbuf = tbuf;
***************
*** 121,127
{
register char *p, *q;
char tcname[16]; /* name of similar terminal */
! char tcbuf[BUFSIZ];
char *holdtbuf = tbuf;
int l;
--- 141,147 -----
#define TERMNAMESIZE 16
char tcname[TERMNAMESIZE]; /* name of similar terminal */
char tcname[16]; /* name of similar terminal */
! char tcbuf[TBUFSIZE];
char *holdtbuf = tbuf;
int l;
***************
*** 135,140
/* p now points to beginning of last field */
if (p[0] != 't' || p[1] != 'c')
return(1);
strcpy(tcname,p+3);
q = tcname;
while (q && *q != ':')
--- 155,161 -----
/* p now points to beginning of last field */
if (p[0] != 't' || p[1] != 'c')
return(1);
+ strncpy(tcname,p+3, TERMNAMESIZE); /* TLH */
strcpy(tcname,p+3);
q = tcname;
while (*q && *q != ':')
***************
*** 137,143
return(1);
strcpy(tcname,p+3);
q = tcname;
! while (q && *q != ':')
q++;
*q = 0;
if (++hopcount > MAXHOP) {
--- 158,164 -----
strncpy(tcname,p+3, TERMNAMESIZE); /* TLH */
strcpy(tcname,p+3);
q = tcname;
! while (*q && *q != ':')
q++;
*q = 0;
if (++hopcount > MAXHOP) {
***************
*** 144,150
write(2, "Infinite tc= loop\n", 18);
return (0);
}
! if (tgetent(tcbuf, tcname) != 1)
return(0);
for (q=tcbuf; *q != ':'; q++)
;
--- 165,171 -----
write(2, "Infinite tc= loop\n", 18);
return (0);
}
! if (_tgetent(tcbuf, tcname) != 1)
return(0);
for (q=tcbuf; *q != ':'; q++)
;
***************
*** 149,155
for (q=tcbuf; *q != ':'; q++)
;
l = p - holdtbuf + strlen(q);
! if (l > BUFSIZ) {
write(2, "Termcap entry too long\n", 23);
q[BUFSIZ - (p-tbuf)] = 0;
}
--- 170,176 -----
for (q=tcbuf; *q != ':'; q++)
;
l = p - holdtbuf + strlen(q);
! if (l > TBUFSIZE) {
write(2, "Termcap entry too long\n", 23);
q[TBUFSIZE - (p-tbuf)] = 0;
}
***************
*** 151,157
l = p - holdtbuf + strlen(q);
if (l > BUFSIZ) {
write(2, "Termcap entry too long\n", 23);
! q[BUFSIZ - (p-tbuf)] = 0;
}
strcpy(p, q+1);
tbuf = holdtbuf;
--- 172,178 -----
l = p - holdtbuf + strlen(q);
if (l > TBUFSIZE) {
write(2, "Termcap entry too long\n", 23);
! q[TBUFSIZE - (p-tbuf)] = 0;
}
strcpy(p, q+1);
tbuf = holdtbuf;hakanson@orstcs.UUCP (hakanson) (05/14/85)
The following is in reference to the termcap (termlib) fixes posted
by pegasus!hansen. Our mailer couldn't find a path to pegasus, so here
is what I would've sent directly if I could have:
I finally got around to examining the patches to termcap(3), and noticed
a few problems with them. The contextual diff seemed to show a few lines
that had been inserted before lines they had been meant to replace (easily
fixed, of course). But the patch to avoid core dumps (doubling the size
of the buffer) only puts off the core dump until an entry gets too big for
the larger buffer. Since I felt this was an inelegant fix, I bit the bullet
and found the bug(s) that caused the problem, and patched that part myself.
I'd appreciate hearing about anything I've broken below. BTW, these
patches work fine on our VAX 750 running 4.2bsd, etc.....
/* rcsdiff -c -r1.1 -r1.2 termcap.c */
RCS file: RCS/termcap.c,v
retrieving revision 1.1
retrieving revision 1.2
diff -c -r1.1 -r1.2
*** /tmp/,RCSt1025137 Tue May 7 12:28:53 1985
--- /tmp/,RCSt2025137 Tue May 7 12:28:57 1985
***************
*** 1,5
#ifndef lint
static char sccsid[] = "@(#)termcap.c 4.1 (Berkeley) 6/27/83";
#endif
#define BUFSIZ 1024
--- 1,6 -----
#ifndef lint
static char sccsid[] = "@(#)termcap.c 4.1 (Berkeley) 6/27/83";
+ static char rcsid[] = "$Header: termcap.c,v 1.2 85/05/06 10:31:50 hakanson Exp $";
#endif
#define BUFSIZ 1024
***************
*** 20,25
* in string capabilities. We don't use stdio because the editor
* doesn't, and because living w/o it is not hard.
*/
static char *tbuf;
static int hopcount; /* detect infinite loops in termcap, init 0 */
--- 21,38 -----
* in string capabilities. We don't use stdio because the editor
* doesn't, and because living w/o it is not hard.
*/
+ /*
+ * Modified 05/06/85 by R. Marion Hakanson (hakanson@oregon-state).
+ * Fixed the following bugs (from Tony Hansen (pegasus!hansen)):
+ *
+ * 1: tgetent() cannot be called more than once in a program (hopcount
+ * doesn't get rezeroed).
+ * 2: an entry with ":tc=foo" but no trailing colon will cause coredumps.
+ * 3. buffer overflows can still occur, causing coredumps.
+ * 4. in tc=foo, foo can only be 16 chars long. this is not checked.
+ *
+ * The patches for #1, #2 & #4 came from hansen; I fixed #3.
+ */
static char *tbuf;
static int hopcount; /* detect infinite loops in termcap, init 0 */
***************
*** 36,41
tgetent(bp, name)
char *bp, *name;
{
register char *cp;
register int c;
register int i = 0, cnt = 0;
--- 49,61 -----
tgetent(bp, name)
char *bp, *name;
{
+ hopcount = 0;
+ return (_tgetent(bp, name));
+ }
+
+ static _tgetent(bp, name)
+ char *bp, *name;
+ {
register char *cp;
register int c;
register int i = 0, cnt = 0;
***************
*** 93,98
break;
}
if (cp >= bp+BUFSIZ) {
write(2,"Termcap entry too long\n", 23);
break;
} else
--- 113,119 -----
break;
}
if (cp >= bp+BUFSIZ) {
+ cp = bp+BUFSIZ-1;
write(2,"Termcap entry too long\n", 23);
break;
} else
***************
*** 117,122
* entries to say "like an HP2621 but doesn't turn on the labels".
* Note that this works because of the left to right scan.
*/
tnchktc()
{
register char *p, *q;
--- 138,145 -----
* entries to say "like an HP2621 but doesn't turn on the labels".
* Note that this works because of the left to right scan.
*/
+ #define NAMESIZE 16
+
tnchktc()
{
register char *p, *q;
***************
*** 120,126
tnchktc()
{
register char *p, *q;
! char tcname[16]; /* name of similar terminal */
char tcbuf[BUFSIZ];
char *holdtbuf = tbuf;
int l;
--- 143,149 -----
tnchktc()
{
register char *p, *q;
! char tcname[NAMESIZE]; /* name of similar terminal */
char tcbuf[BUFSIZ];
char *holdtbuf = tbuf;
int l;
***************
*** 135,141
/* p now points to beginning of last field */
if (p[0] != 't' || p[1] != 'c')
return(1);
! strcpy(tcname,p+3);
q = tcname;
while (q && *q != ':')
q++;
--- 158,165 -----
/* p now points to beginning of last field */
if (p[0] != 't' || p[1] != 'c')
return(1);
! strncpy(tcname,p+3,NAMESIZE);
! tcname[NAMESIZE-1] = 0;
q = tcname;
while (*q && *q != ':')
q++;
***************
*** 137,143
return(1);
strcpy(tcname,p+3);
q = tcname;
! while (q && *q != ':')
q++;
*q = 0;
if (++hopcount > MAXHOP) {
--- 161,167 -----
strncpy(tcname,p+3,NAMESIZE);
tcname[NAMESIZE-1] = 0;
q = tcname;
! while (*q && *q != ':')
q++;
*q = 0;
if (++hopcount > MAXHOP) {
***************
*** 144,150
write(2, "Infinite tc= loop\n", 18);
return (0);
}
! if (tgetent(tcbuf, tcname) != 1)
return(0);
for (q=tcbuf; *q != ':'; q++)
;
--- 168,174 -----
write(2, "Infinite tc= loop\n", 18);
return (0);
}
! if (_tgetent(tcbuf, tcname) != 1)
return(0);
for (q=tcbuf; *q != ':'; q++)
;
***************
*** 151,157
l = p - holdtbuf + strlen(q);
if (l > BUFSIZ) {
write(2, "Termcap entry too long\n", 23);
! q[BUFSIZ - (p-tbuf)] = 0;
}
strcpy(p, q+1);
tbuf = holdtbuf;
--- 175,181 -----
l = p - holdtbuf + strlen(q);
if (l > BUFSIZ) {
write(2, "Termcap entry too long\n", 23);
! q[BUFSIZ - (p-holdtbuf)] = 0;
}
strcpy(p, q+1);
tbuf = holdtbuf;
/* end diff listing */
Marion Hakanson CSnet: hakanson%oregon-state@csnet-relay
UUCP : {hp-pcd,tekchips}!orstcs!hakanson