* Buffer-safe string function cleanup. There are a couple of strcpy()

and strcat()s which would be more difficult to fix, but I think they're
  safe anyway.
* Don't crash at runtime by overflowing a buffer with constant data in
  print-icmp.c on a long hostname.
* Don't overflow a static buffer by trying to decode an AFS ACL into a buffer
  which is way too small for it.

Reviewed by:    -audit
This commit is contained in:
Kris Kennaway 2000-10-05 02:49:49 +00:00
parent 1e3661ab60
commit 7524a0790d
5 changed files with 32 additions and 28 deletions

@ -559,7 +559,7 @@ tcpport_string(u_short port)
tp->addr = i; tp->addr = i;
tp->nxt = newhnamemem(); tp->nxt = newhnamemem();
(void)sprintf(buf, "%u", i); (void)snprintf(buf, sizeof(buf), "%u", i);
tp->name = savestr(buf); tp->name = savestr(buf);
return (tp->name); return (tp->name);
} }
@ -578,7 +578,7 @@ udpport_string(register u_short port)
tp->addr = i; tp->addr = i;
tp->nxt = newhnamemem(); tp->nxt = newhnamemem();
(void)sprintf(buf, "%u", i); (void)snprintf(buf, sizeof(buf), "%u", i);
tp->name = savestr(buf); tp->name = savestr(buf);
return (tp->name); return (tp->name);
} }
@ -604,7 +604,7 @@ init_servarray(void)
while (table->name) while (table->name)
table = table->nxt; table = table->nxt;
if (nflag) { if (nflag) {
(void)sprintf(buf, "%d", port); (void)snprintf(buf, sizeof(buf), "%d", port);
table->name = savestr(buf); table->name = savestr(buf);
} else } else
table->name = savestr(sv->s_name); table->name = savestr(sv->s_name);

@ -500,7 +500,7 @@ ataddr_string(u_short atnet, u_char athost)
{ {
register struct hnamemem *tp, *tp2; register struct hnamemem *tp, *tp2;
register int i = (atnet << 8) | athost; register int i = (atnet << 8) | athost;
char nambuf[256]; char nambuf[MAXHOSTNAMELEN + 20];
static int first = 1; static int first = 1;
FILE *fp; FILE *fp;
@ -545,7 +545,7 @@ ataddr_string(u_short atnet, u_char athost)
if (tp2->addr == i) { if (tp2->addr == i) {
tp->addr = (atnet << 8) | athost; tp->addr = (atnet << 8) | athost;
tp->nxt = newhnamemem(); tp->nxt = newhnamemem();
(void)sprintf(nambuf, "%s.%d", tp2->name, athost); (void)snprintf(nambuf, sizeof(nambuf), "%s.%d", tp2->name, athost);
tp->name = savestr(nambuf); tp->name = savestr(nambuf);
return (tp->name); return (tp->name);
} }

@ -17,6 +17,8 @@
* THIS SOFTWARE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR IMPLIED * THIS SOFTWARE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR IMPLIED
* WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED WARRANTIES OF * WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED WARRANTIES OF
* MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE. * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE.
*
* $FreeBSD$
*/ */
#ifndef lint #ifndef lint
@ -395,12 +397,12 @@ q933_print(const u_char *p, int length)
break; break;
case LINK_VERIFY_IE_91: case LINK_VERIFY_IE_91:
case LINK_VERIFY_IE_94: case LINK_VERIFY_IE_94:
sprintf(temp_str,"TX Seq: %3d, RX Seq: %3d", snprintf(temp_str, sizeof(temp_str), "TX Seq: %3d, RX Seq: %3d",
ptemp[2], ptemp[3]); ptemp[2], ptemp[3]);
decode_str = temp_str; decode_str = temp_str;
break; break;
case PVC_STATUS_IE: case PVC_STATUS_IE:
sprintf(temp_str,"DLCI %d: status %s %s", snprintf(temp_str,sizeof(temp_str), "DLCI %d: status %s %s",
((ptemp[2]&0x3f)<<4)+ ((ptemp[3]&0x78)>>3), ((ptemp[2]&0x3f)<<4)+ ((ptemp[3]&0x78)>>3),
ptemp[4] & 0x8 ?"new,":" ", ptemp[4] & 0x8 ?"new,":" ",
ptemp[4] & 0x2 ?"Active":"Inactive"); ptemp[4] & 0x2 ?"Active":"Inactive");

@ -177,7 +177,7 @@ icmp_print(register const u_char *bp, u_int plen, register const u_char *bp2)
register const struct ip *oip; register const struct ip *oip;
register const struct udphdr *ouh; register const struct udphdr *ouh;
register u_int hlen, dport, mtu; register u_int hlen, dport, mtu;
char buf[256]; char buf[MAXHOSTNAMELEN + 100];
dp = (struct icmp *)bp; dp = (struct icmp *)bp;
ip = (struct ip *)bp2; ip = (struct ip *)bp2;
@ -198,7 +198,7 @@ icmp_print(register const u_char *bp, u_int plen, register const u_char *bp2)
case ICMP_UNREACH_PROTOCOL: case ICMP_UNREACH_PROTOCOL:
TCHECK(dp->icmp_ip.ip_p); TCHECK(dp->icmp_ip.ip_p);
(void)sprintf(buf, "%s protocol %d unreachable", (void)snprintf(buf, sizeof(buf), "%s protocol %d unreachable",
ipaddr_string(&dp->icmp_ip.ip_dst), ipaddr_string(&dp->icmp_ip.ip_dst),
dp->icmp_ip.ip_p); dp->icmp_ip.ip_p);
break; break;
@ -212,21 +212,21 @@ icmp_print(register const u_char *bp, u_int plen, register const u_char *bp2)
switch (oip->ip_p) { switch (oip->ip_p) {
case IPPROTO_TCP: case IPPROTO_TCP:
(void)sprintf(buf, (void)snprintf(buf, sizeof(buf),
"%s tcp port %s unreachable", "%s tcp port %s unreachable",
ipaddr_string(&oip->ip_dst), ipaddr_string(&oip->ip_dst),
tcpport_string(dport)); tcpport_string(dport));
break; break;
case IPPROTO_UDP: case IPPROTO_UDP:
(void)sprintf(buf, (void)snprintf(buf, sizeof(buf),
"%s udp port %s unreachable", "%s udp port %s unreachable",
ipaddr_string(&oip->ip_dst), ipaddr_string(&oip->ip_dst),
udpport_string(dport)); udpport_string(dport));
break; break;
default: default:
(void)sprintf(buf, (void)snprintf(buf, sizeof(buf),
"%s protocol %d port %d unreachable", "%s protocol %d port %d unreachable",
ipaddr_string(&oip->ip_dst), ipaddr_string(&oip->ip_dst),
oip->ip_p, dport); oip->ip_p, dport);
@ -241,11 +241,11 @@ icmp_print(register const u_char *bp, u_int plen, register const u_char *bp2)
mp = (struct mtu_discovery *)&dp->icmp_void; mp = (struct mtu_discovery *)&dp->icmp_void;
mtu = EXTRACT_16BITS(&mp->nexthopmtu); mtu = EXTRACT_16BITS(&mp->nexthopmtu);
if (mtu) if (mtu)
(void)sprintf(buf, (void)snprintf(buf, sizeof(buf),
"%s unreachable - need to frag (mtu %d)", "%s unreachable - need to frag (mtu %d)",
ipaddr_string(&dp->icmp_ip.ip_dst), mtu); ipaddr_string(&dp->icmp_ip.ip_dst), mtu);
else else
(void)sprintf(buf, (void)snprintf(buf, sizeof(buf),
"%s unreachable - need to frag", "%s unreachable - need to frag",
ipaddr_string(&dp->icmp_ip.ip_dst)); ipaddr_string(&dp->icmp_ip.ip_dst));
} }
@ -254,7 +254,7 @@ icmp_print(register const u_char *bp, u_int plen, register const u_char *bp2)
default: default:
fmt = tok2str(unreach2str, "#%d %%s unreachable", fmt = tok2str(unreach2str, "#%d %%s unreachable",
dp->icmp_code); dp->icmp_code);
(void)sprintf(buf, fmt, (void)snprintf(buf, sizeof(buf), fmt,
ipaddr_string(&dp->icmp_ip.ip_dst)); ipaddr_string(&dp->icmp_ip.ip_dst));
break; break;
} }
@ -264,7 +264,7 @@ icmp_print(register const u_char *bp, u_int plen, register const u_char *bp2)
TCHECK(dp->icmp_ip.ip_dst); TCHECK(dp->icmp_ip.ip_dst);
fmt = tok2str(type2str, "redirect-#%d %%s to net %%s", fmt = tok2str(type2str, "redirect-#%d %%s to net %%s",
dp->icmp_code); dp->icmp_code);
(void)sprintf(buf, fmt, (void)snprintf(buf, sizeof(buf), fmt,
ipaddr_string(&dp->icmp_ip.ip_dst), ipaddr_string(&dp->icmp_ip.ip_dst),
ipaddr_string(&dp->icmp_gwaddr)); ipaddr_string(&dp->icmp_gwaddr));
break; break;
@ -284,30 +284,30 @@ icmp_print(register const u_char *bp, u_int plen, register const u_char *bp2)
cp = buf + strlen(buf); cp = buf + strlen(buf);
lifetime = EXTRACT_16BITS(&ihp->ird_lifetime); lifetime = EXTRACT_16BITS(&ihp->ird_lifetime);
if (lifetime < 60) if (lifetime < 60)
(void)sprintf(cp, "%u", lifetime); (void)snprintf(cp, sizeof(buf) - strlen(buf), "%u", lifetime);
else if (lifetime < 60 * 60) else if (lifetime < 60 * 60)
(void)sprintf(cp, "%u:%02u", (void)snprintf(cp, sizeof(buf) - strlen(buf), "%u:%02u",
lifetime / 60, lifetime % 60); lifetime / 60, lifetime % 60);
else else
(void)sprintf(cp, "%u:%02u:%02u", (void)snprintf(cp, sizeof(buf) - strlen(buf), "%u:%02u:%02u",
lifetime / 3600, lifetime / 3600,
(lifetime % 3600) / 60, (lifetime % 3600) / 60,
lifetime % 60); lifetime % 60);
cp = buf + strlen(buf); cp = buf + strlen(buf);
num = ihp->ird_addrnum; num = ihp->ird_addrnum;
(void)sprintf(cp, " %d:", num); (void)snprintf(cp, sizeof(buf) - strlen(buf), " %d:", num);
cp = buf + strlen(buf); cp = buf + strlen(buf);
size = ihp->ird_addrsiz; size = ihp->ird_addrsiz;
if (size != 2) { if (size != 2) {
(void)sprintf(cp, " [size %d]", size); (void)snprintf(cp, sizeof(buf) - strlen(buf), " [size %d]", size);
break; break;
} }
idp = (struct id_rdiscovery *)&dp->icmp_data; idp = (struct id_rdiscovery *)&dp->icmp_data;
while (num-- > 0) { while (num-- > 0) {
TCHECK(*idp); TCHECK(*idp);
(void)sprintf(cp, " {%s %u}", (void)snprintf(cp, sizeof(buf) - strlen(buf), " {%s %u}",
ipaddr_string(&idp->ird_addr), ipaddr_string(&idp->ird_addr),
EXTRACT_32BITS(&idp->ird_pref)); EXTRACT_32BITS(&idp->ird_pref));
cp = buf + strlen(buf); cp = buf + strlen(buf);
@ -328,25 +328,25 @@ icmp_print(register const u_char *bp, u_int plen, register const u_char *bp2)
break; break;
default: default:
(void)sprintf(buf, "time exceeded-#%d", dp->icmp_code); (void)snprintf(buf, sizeof(buf), "time exceeded-#%d", dp->icmp_code);
break; break;
} }
break; break;
case ICMP_PARAMPROB: case ICMP_PARAMPROB:
if (dp->icmp_code) if (dp->icmp_code)
(void)sprintf(buf, "parameter problem - code %d", (void)snprintf(buf, sizeof(buf), "parameter problem - code %d",
dp->icmp_code); dp->icmp_code);
else { else {
TCHECK(dp->icmp_pptr); TCHECK(dp->icmp_pptr);
(void)sprintf(buf, "parameter problem - octet %d", (void)snprintf(buf, sizeof(buf), "parameter problem - octet %d",
dp->icmp_pptr); dp->icmp_pptr);
} }
break; break;
case ICMP_MASKREPLY: case ICMP_MASKREPLY:
TCHECK(dp->icmp_mask); TCHECK(dp->icmp_mask);
(void)sprintf(buf, "address mask is 0x%08x", (void)snprintf(buf, sizeof(buf), "address mask is 0x%08x",
(u_int32_t)ntohl(dp->icmp_mask)); (u_int32_t)ntohl(dp->icmp_mask));
break; break;

@ -132,7 +132,9 @@ progstr(prog)
rp = getrpcbynumber(prog); rp = getrpcbynumber(prog);
if (rp == NULL) if (rp == NULL)
(void) sprintf(buf, "#%u", prog); (void) sprintf(buf, "#%u", prog);
else else {
strcpy(buf, rp->r_name); strncpy(buf, rp->r_name, sizeof(buf)-1);
buf[sizeof(buf)-1] = '\0';
}
return (buf); return (buf);
} }