From owner-freebsd-audit Wed Sep 27 21:25:14 2000 Delivered-To: freebsd-audit@freebsd.org Received: from freefall.freebsd.org (freefall.FreeBSD.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id EAA5237B424; Wed, 27 Sep 2000 21:24:10 -0700 (PDT) Received: from localhost (kris@localhost) by freefall.freebsd.org (8.9.3/8.9.2) with ESMTP id VAA35398; Wed, 27 Sep 2000 21:24:10 -0700 (PDT) (envelope-from kris@FreeBSD.org) X-Authentication-Warning: freefall.freebsd.org: kris owned process doing -bs Date: Wed, 27 Sep 2000 21:24:10 -0700 (PDT) From: Kris Kennaway To: freebsd-audit@freebsd.org, patches@tcpdump.org Subject: tcpdump security vulnerabilities Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG 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 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message