Date: Wed, 27 Sep 2000 21:24:10 -0700 (PDT) From: Kris Kennaway <kris@FreeBSD.org> To: freebsd-audit@freebsd.org, patches@tcpdump.org Subject: tcpdump security vulnerabilities Message-ID: <Pine.BSF.4.21.0009272116480.31282-100000@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
Hi,
I happened to be taking a look through the tcpdump 3.5 source tonight and
noticed several obvious bugs which allow a remote user to crash the local
tcpdump - for example, print_icmp.c tries to sprintf() into a buffer of
length 256 including the resolved remote hostname which was the source of
the ICMP packet - but 256 is also the maximum length of the hostname which
is returned by the resolver. Therefore we can overflow the static buffer
with about 10-20 bytes of constant text (the rest of the intended message)
- this crashes tcpdump, but I dont think it is likely exploitable. It
would however be useful as an attack against tcpdump being used as an IDS
to watch for malicious network activity.
Looking further, I found what appears to be a vulnerability in print_rx.c
allowing execution of arbitrary code on the local system - acl_print()
will attempt to decode an AFS ACL packet - this packet may be as large as
1024 bytes, yet it is sscanf()'ed into a 128-byte static buffer. Thus the
attacker has about 896 bytes to play with.
There are still a couple of strcpy()/strcat() operations left in the
source, but these seemed to be pretty safe and they were difficult to
patch.
Please review this patch - if this is acceptable to the tcpdump guys, I'll
commit it to FreeBSD and release an advisory shortly thereafter.
Kris
Index: addrtoname.c
===================================================================
RCS file: /mnt/ncvs/src/contrib/tcpdump/addrtoname.c,v
retrieving revision 1.7
diff -u -r1.7 addrtoname.c
--- addrtoname.c 2000/03/08 02:24:10 1.7
+++ addrtoname.c 2000/09/28 03:51:39
@@ -559,7 +559,7 @@
tp->addr = i;
tp->nxt = newhnamemem();
- (void)sprintf(buf, "%u", i);
+ (void)snprintf(buf, sizeof(buf), "%u", i);
tp->name = savestr(buf);
return (tp->name);
}
@@ -578,7 +578,7 @@
tp->addr = i;
tp->nxt = newhnamemem();
- (void)sprintf(buf, "%u", i);
+ (void)snprintf(buf, sizeof(buf), "%u", i);
tp->name = savestr(buf);
return (tp->name);
}
@@ -604,7 +604,7 @@
while (table->name)
table = table->nxt;
if (nflag) {
- (void)sprintf(buf, "%d", port);
+ (void)snprintf(buf, sizeof(buf), "%d", port);
table->name = savestr(buf);
} else
table->name = savestr(sv->s_name);
Index: print-atalk.c
===================================================================
RCS file: /mnt/ncvs/src/contrib/tcpdump/print-atalk.c,v
retrieving revision 1.7
diff -u -r1.7 print-atalk.c
--- print-atalk.c 2000/01/30 01:00:51 1.7
+++ print-atalk.c 2000/09/28 03:55:28
@@ -500,7 +500,7 @@
{
register struct hnamemem *tp, *tp2;
register int i = (atnet << 8) | athost;
- char nambuf[256];
+ char nambuf[MAXHOSTNAMELEN + 20];
static int first = 1;
FILE *fp;
@@ -545,7 +545,7 @@
if (tp2->addr == i) {
tp->addr = (atnet << 8) | athost;
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);
return (tp->name);
}
Index: print-bgp.c
===================================================================
RCS file: /mnt/ncvs/src/contrib/tcpdump/print-bgp.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 print-bgp.c
--- print-bgp.c 2000/01/30 00:45:33 1.1.1.1
+++ print-bgp.c 2000/09/28 03:49:57
@@ -240,7 +240,7 @@
{
static char buf[20];
if (value < 0 || siz <= value || table[value] == NULL) {
- sprintf(buf, "#%d", value);
+ snprintf(buf, sizeof(buf), "#%d", value);
return buf;
} else
return table[value];
@@ -266,7 +266,7 @@
} else
p = NULL;
if (p == NULL) {
- sprintf(buf, "#%d", minor);
+ snprintf(buf, sizeof(buf), "#%d", minor);
return buf;
} else
return p;
@@ -288,7 +288,7 @@
((u_char *)&addr)[(plen + 7) / 8 - 1] &=
((0xff00 >> (plen % 8)) & 0xff);
}
- sprintf(buf, "%s/%d", getname((char *)&addr), plen);
+ snprintf(buf, buflen, "%s/%d", getname((char *)&addr), plen);
return 1 + (plen + 7) / 8;
}
@@ -309,7 +309,7 @@
addr.s6_addr[(plen + 7) / 8 - 1] &=
((0xff00 >> (plen % 8)) & 0xff);
}
- sprintf(buf, "%s/%d", getname6((char *)&addr), plen);
+ snprintf(buf, buflen, "%s/%d", getname6((char *)&addr), plen);
return 1 + (plen + 7) / 8;
}
#endif
@@ -323,7 +323,7 @@
int advance;
int tlen;
const u_char *p;
- char buf[256];
+ char buf[MAXHOSTNAMELEN + 100];
p = dat;
@@ -608,7 +608,7 @@
if (dat + length > p) {
printf("(NLRI:"); /* ) */
while (dat + length > p) {
- char buf[256];
+ char buf[MAXHOSTNAMELEN + 100];
i = decode_prefix4(p, buf, sizeof(buf));
printf(" %s", buf);
if (i < 0)
Index: print-fr.c
===================================================================
RCS file: /mnt/ncvs/src/contrib/tcpdump/print-fr.c,v
retrieving revision 1.2
diff -u -r1.2 print-fr.c
--- print-fr.c 1998/01/01 04:13:43 1.2
+++ print-fr.c 2000/09/28 03:54:40
@@ -395,12 +395,12 @@
break;
case LINK_VERIFY_IE_91:
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]);
decode_str = temp_str;
break;
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[4] & 0x8 ?"new,":" ",
ptemp[4] & 0x2 ?"Active":"Inactive");
Index: print-icmp.c
===================================================================
RCS file: /mnt/ncvs/src/contrib/tcpdump/print-icmp.c,v
retrieving revision 1.4
diff -u -r1.4 print-icmp.c
--- print-icmp.c 2000/01/30 01:00:52 1.4
+++ print-icmp.c 2000/09/28 03:49:57
@@ -177,7 +177,7 @@
register const struct ip *oip;
register const struct udphdr *ouh;
register u_int hlen, dport, mtu;
- char buf[256];
+ char buf[MAXHOSTNAMELEN + 100];
dp = (struct icmp *)bp;
ip = (struct ip *)bp2;
@@ -198,7 +198,7 @@
case ICMP_UNREACH_PROTOCOL:
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),
dp->icmp_ip.ip_p);
break;
@@ -212,21 +212,21 @@
switch (oip->ip_p) {
case IPPROTO_TCP:
- (void)sprintf(buf,
+ (void)snprintf(buf, sizeof(buf),
"%s tcp port %s unreachable",
ipaddr_string(&oip->ip_dst),
tcpport_string(dport));
break;
case IPPROTO_UDP:
- (void)sprintf(buf,
+ (void)snprintf(buf, sizeof(buf),
"%s udp port %s unreachable",
ipaddr_string(&oip->ip_dst),
udpport_string(dport));
break;
default:
- (void)sprintf(buf,
+ (void)snprintf(buf, sizeof(buf),
"%s protocol %d port %d unreachable",
ipaddr_string(&oip->ip_dst),
oip->ip_p, dport);
@@ -241,11 +241,11 @@
mp = (struct mtu_discovery *)&dp->icmp_void;
mtu = EXTRACT_16BITS(&mp->nexthopmtu);
if (mtu)
- (void)sprintf(buf,
+ (void)snprintf(buf, sizeof(buf),
"%s unreachable - need to frag (mtu %d)",
ipaddr_string(&dp->icmp_ip.ip_dst), mtu);
else
- (void)sprintf(buf,
+ (void)snprintf(buf, sizeof(buf),
"%s unreachable - need to frag",
ipaddr_string(&dp->icmp_ip.ip_dst));
}
@@ -254,7 +254,7 @@
default:
fmt = tok2str(unreach2str, "#%d %%s unreachable",
dp->icmp_code);
- (void)sprintf(buf, fmt,
+ (void)snprintf(buf, sizeof(buf), fmt,
ipaddr_string(&dp->icmp_ip.ip_dst));
break;
}
@@ -264,7 +264,7 @@
TCHECK(dp->icmp_ip.ip_dst);
fmt = tok2str(type2str, "redirect-#%d %%s to net %%s",
dp->icmp_code);
- (void)sprintf(buf, fmt,
+ (void)snprintf(buf, sizeof(buf), fmt,
ipaddr_string(&dp->icmp_ip.ip_dst),
ipaddr_string(&dp->icmp_gwaddr));
break;
@@ -284,30 +284,30 @@
cp = buf + strlen(buf);
lifetime = EXTRACT_16BITS(&ihp->ird_lifetime);
if (lifetime < 60)
- (void)sprintf(cp, "%u", lifetime);
+ (void)snprintf(cp, sizeof(buf) - strlen(buf), "%u", lifetime);
else if (lifetime < 60 * 60)
- (void)sprintf(cp, "%u:%02u",
+ (void)snprintf(cp, sizeof(buf) - strlen(buf), "%u:%02u",
lifetime / 60, lifetime % 60);
else
- (void)sprintf(cp, "%u:%02u:%02u",
+ (void)snprintf(cp, sizeof(buf) - strlen(buf), "%u:%02u:%02u",
lifetime / 3600,
(lifetime % 3600) / 60,
lifetime % 60);
cp = buf + strlen(buf);
num = ihp->ird_addrnum;
- (void)sprintf(cp, " %d:", num);
+ (void)snprintf(cp, sizeof(buf) - strlen(buf), " %d:", num);
cp = buf + strlen(buf);
size = ihp->ird_addrsiz;
if (size != 2) {
- (void)sprintf(cp, " [size %d]", size);
+ (void)snprintf(cp, sizeof(buf) - strlen(buf), " [size %d]", size);
break;
}
idp = (struct id_rdiscovery *)&dp->icmp_data;
while (num-- > 0) {
TCHECK(*idp);
- (void)sprintf(cp, " {%s %u}",
+ (void)snprintf(cp, sizeof(buf) - strlen(buf), " {%s %u}",
ipaddr_string(&idp->ird_addr),
EXTRACT_32BITS(&idp->ird_pref));
cp = buf + strlen(buf);
@@ -328,25 +328,25 @@
break;
default:
- (void)sprintf(buf, "time exceeded-#%d", dp->icmp_code);
+ (void)snprintf(buf, sizeof(buf), "time exceeded-#%d", dp->icmp_code);
break;
}
break;
case ICMP_PARAMPROB:
if (dp->icmp_code)
- (void)sprintf(buf, "parameter problem - code %d",
+ (void)snprintf(buf, sizeof(buf), "parameter problem - code %d",
dp->icmp_code);
else {
TCHECK(dp->icmp_pptr);
- (void)sprintf(buf, "parameter problem - octet %d",
+ (void)snprintf(buf, sizeof(buf), "parameter problem - octet %d",
dp->icmp_pptr);
}
break;
case ICMP_MASKREPLY:
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));
break;
Index: print-rx.c
===================================================================
RCS file: /mnt/ncvs/src/contrib/tcpdump/print-rx.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 print-rx.c
--- print-rx.c 2000/01/30 00:45:46 1.1.1.1
+++ print-rx.c 2000/09/28 04:13:59
@@ -341,7 +341,7 @@
static void fs_print(const u_char *, int);
static void fs_reply_print(const u_char *, int, int32_t);
-static void acl_print(u_char *, u_char *);
+static void acl_print(u_char *, int, u_char *);
static void cb_print(const u_char *, int);
static void cb_reply_print(const u_char *, int, int32_t);
static void prot_print(const u_char *, int);
@@ -754,7 +754,7 @@
TRUNC(i);
strncpy(a, bp, min(AFSOPAQUEMAX, i));
a[i] = '\0';
- acl_print((u_char *) a, (u_char *) a + i);
+ acl_print((u_char *) a, sizeof(a), (u_char *) a + i);
break;
}
case 137: /* Create file */
@@ -865,7 +865,7 @@
TRUNC(i);
strncpy(a, bp, min(AFSOPAQUEMAX, i));
a[i] = '\0';
- acl_print((u_char *) a, (u_char *) a + i);
+ acl_print((u_char *) a, sizeof(a), (u_char *) a + i);
break;
}
case 137: /* Create file */
@@ -912,19 +912,22 @@
*/
static void
-acl_print(u_char *s, u_char *end)
+acl_print(u_char *s, int maxsize, u_char *end)
{
int pos, neg, acl;
int n, i;
- char user[128];
+ char *user;
- if (sscanf((char *) s, "%d %d\n%n", &pos, &neg, &n) != 2)
+ if ((user = (char *)malloc(maxsize)) == NULL)
return;
+
+ if (sscanf((char *) s, "%d %d\n%n", &pos, &neg, &n) != 2)
+ goto finish;
s += n;
if (s > end)
- return;
+ goto finish;
/*
* This wacky order preserves the order used by the "fs" command
@@ -948,25 +951,29 @@
for (i = 0; i < pos; i++) {
if (sscanf((char *) s, "%s %d\n%n", user, &acl, &n) != 2)
- return;
+ goto finish;
s += n;
printf(" +{%s ", user);
ACLOUT(acl);
printf("}");
if (s > end)
- return;
+ goto finish;
}
for (i = 0; i < neg; i++) {
if (sscanf((char *) s, "%s %d\n%n", user, &acl, &n) != 2)
- return;
+ goto finish;
s += n;
printf(" -{%s ", user);
ACLOUT(acl);
printf("}");
if (s > end)
- return;
+ goto finish;
}
+
+finish:
+ free(user);
+ return;
}
#undef ACLOUT
Index: print-sunrpc.c
===================================================================
RCS file: /mnt/ncvs/src/contrib/tcpdump/print-sunrpc.c,v
retrieving revision 1.5
diff -u -r1.5 print-sunrpc.c
--- print-sunrpc.c 2000/01/30 01:00:54 1.5
+++ print-sunrpc.c 2000/09/28 04:08:33
@@ -132,7 +132,9 @@
rp = getrpcbynumber(prog);
if (rp == NULL)
(void) sprintf(buf, "#%u", prog);
- else
- strcpy(buf, rp->r_name);
+ else {
+ strncpy(buf, rp->r_name, sizeof(buf)-1);
+ buf[sizeof(buf)-1] = '\0';
+ }
return (buf);
}
Index: print-telnet.c
===================================================================
RCS file: /mnt/ncvs/src/contrib/tcpdump/print-telnet.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 print-telnet.c
--- print-telnet.c 2000/01/30 00:45:48 1.1.1.1
+++ print-telnet.c 2000/09/28 03:49:58
@@ -128,10 +128,10 @@
x = *sp++; /* option */
length--;
if (x >= 0 && x < NTELOPTS) {
- (void)sprintf(tnet, "%s %s",
+ (void)snprintf(tnet, sizeof(tnet), "%s %s",
telcmds[i], telopts[x]);
} else {
- (void)sprintf(tnet, "%s %#x",
+ (void)snprintf(tnet, sizeof(tnet), "%s %#x",
telcmds[i], x);
}
break;
Index: smbutil.c
===================================================================
RCS file: /mnt/ncvs/src/contrib/tcpdump/smbutil.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 smbutil.c
--- smbutil.c 2000/01/30 00:45:52 1.1.1.1
+++ smbutil.c 2000/09/28 03:49:58
@@ -680,17 +680,17 @@
for (j=0;err[j].name;j++)
if (num == err[j].code)
{
- sprintf(ret,"%s - %s (%s)",err_classes[i].class,
+ snprintf(ret, sizeof(ret), "%s - %s (%s)",err_classes[i].class,
err[j].name,err[j].message);
return ret;
}
}
- sprintf(ret,"%s - %d",err_classes[i].class,num);
+ snprintf(ret, sizeof(ret), "%s - %d",err_classes[i].class,num);
return ret;
}
- sprintf(ret,"ERROR: Unknown error (%d,%d)",class,num);
+ snprintf(ret, sizeof(ret), "ERROR: Unknown error (%d,%d)",class,num);
return(ret);
}
Index: util.c
===================================================================
RCS file: /mnt/ncvs/src/contrib/tcpdump/util.c,v
retrieving revision 1.1.1.4
diff -u -r1.1.1.4 util.c
--- util.c 2000/01/30 00:45:54 1.1.1.4
+++ util.c 2000/09/28 03:49:58
@@ -205,7 +205,7 @@
}
if (fmt == NULL)
fmt = "#%d";
- (void)sprintf(buf, fmt, v);
+ (void)snprintf(buf, sizeof(buf), fmt, v);
return (buf);
}
--
In God we Trust -- all others must submit an X.509 certificate.
-- Charles Forsythe <forsythe@alum.mit.edu>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0009272116480.31282-100000>