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 From owner-freebsd-audit Thu Sep 28 7:35:21 2000 Delivered-To: freebsd-audit@freebsd.org Received: from spirit.jaded.net (shortbus.jaded.net [216.94.132.8]) by hub.freebsd.org (Postfix) with ESMTP id 3E76137B422; Thu, 28 Sep 2000 07:35:17 -0700 (PDT) Received: (from dan@localhost) by spirit.jaded.net (8.9.3/8.9.3) id KAA38156; Thu, 28 Sep 2000 10:35:42 -0400 (EDT) (envelope-from dan) Date: Thu, 28 Sep 2000 10:35:42 -0400 From: Dan Moschuk To: Kris Kennaway Cc: audit@FreeBSD.org Subject: Re: tcpdump security vulnerabilities Message-ID: <20000928103542.A38089@spirit.jaded.net> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: ; from kris@FreeBSD.org on Wed, Sep 27, 2000 at 09:24:10PM -0700 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 [ snip ] | Please review this patch - if this is acceptable to the tcpdump guys, I'll | commit it to FreeBSD and release an advisory shortly thereafter. The patch seems fairly harmless, but remember that tcpdump is contrib code and the patch should go the maintainers first. To increase the chance of them accepting it, you may want to roll your own snprintf() routine for the few remaining OSs that don't have it, or hint that they should do it if they want their code to compile on older versions of Solaris. :) Cheers! -Dan -- Man is a rational animal who always loses his temper when he is called upon to act in accordance with the dictates of reason. -- Oscar Wilde To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Thu Sep 28 9:11:29 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 E9A9237B42C; Thu, 28 Sep 2000 09:11:27 -0700 (PDT) Received: from localhost (kris@localhost) by freefall.freebsd.org (8.9.3/8.9.2) with ESMTP id JAA97775; Thu, 28 Sep 2000 09:11:27 -0700 (PDT) (envelope-from kris@FreeBSD.org) X-Authentication-Warning: freefall.freebsd.org: kris owned process doing -bs Date: Thu, 28 Sep 2000 09:11:27 -0700 (PDT) From: Kris Kennaway To: Dan Moschuk Cc: audit@FreeBSD.org Subject: Re: tcpdump security vulnerabilities In-Reply-To: <20000928103542.A38089@spirit.jaded.net> 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 On Thu, 28 Sep 2000, Dan Moschuk wrote: > The patch seems fairly harmless, but remember that tcpdump is contrib > code and the patch should go the maintainers first. To increase the chance > of them accepting it, you may want to roll your own snprintf() routine for > the few remaining OSs that don't have it, or hint that they should do it > if they want their code to compile on older versions of Solaris. :) It was CC'ed to the tcpdump "patches submission" address. They use snprintf in the rest of the code, so I assume that it's taken care of. Kris -- 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 From owner-freebsd-audit Fri Sep 29 18:15: 9 2000 Delivered-To: freebsd-audit@freebsd.org Received: from fw.wintelcom.net (ns1.wintelcom.net [209.1.153.20]) by hub.freebsd.org (Postfix) with ESMTP id 6AB1037B503 for ; Fri, 29 Sep 2000 18:15:07 -0700 (PDT) Received: (from bright@localhost) by fw.wintelcom.net (8.10.0/8.10.0) id e8U1F7f24324 for freebsd-audit@freebsd.org; Fri, 29 Sep 2000 18:15:07 -0700 (PDT) Date: Fri, 29 Sep 2000 18:15:07 -0700 From: Alfred Perlstein To: freebsd-audit@freebsd.org Subject: ptrace may have a security flaw with reparenting Message-ID: <20000929181506.W27736@fw.wintelcom.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.4i Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG I haven't had time to test nor do a complete audit, but a pretty good glance at the ptrace code makes it look like one can attach a process to another process. Here's how it looks like it may be possible: pid 100 forks child 101 pid X ptracte attaches to 101 pid X waits for pid 100 to exit and another unrelated (target) process to get the pid via wraparound pid X then detaches pid 101 is now attached to 100 but it shouldn't be! I'm not aware of any special capabilities gained from just being a child process of another process, however the simple problem is that now the false child can subvert the parent by exiting and leaving a return value that would confuse the parent. Assume setuid app 'foo' forks a child to do authentication and relies on the child returning success for positive auth, well now you have a race window in which to return early and trick the parent. Again even if this is possible it's probably going to be really obscure and difficult to exploit, however after someone says that on BUGTRAQ it's usually a few hours (days tops) before someone actually finds a case where it is possible. My apologies if this isn't possible (there may be other checks in the code to disallow this trick), but I just don't have time to research it further. thanks, -- -Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org] "I have the heart of a child; I keep it in a jar on my desk." To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message