Fix a very stupid heap corruption bug: in ypproc_match_2_svc(), when

we decide to do a DNS lookup, we NUL terminate the key string provided
by the client before passing it into the DNS lookup module. This is
actually wrong. Assume the key is 'foo.com'. In this case, key.keydat_val
will be "foo.com" and key.keydat_len will be 7 (seven characters; the
string is not NUL-terminated so it is not 8 as you might expect).
The string "foo.com" is actually allocated by the XDR routines when the
RPC request is decoded; exactly 7 bytes are allocated. By adding a NUL,
the string becomes "foo.com\0", but the '\0' goes into an 8th byte which
was never allocated for this string and which could be anywhere. The result
is that while the initial request may succeed, we could trash other
dynamically allocated structures (like, oh, I dunno, the circular map
cache queue?) and SEGV later. This is in fact what happens.

The fix is to copy the string into a larger local buffer and NUL-terminate
that buffer instead.

Crash first reported by: Ricky Chan <ricky@come.net.uk>
Bug finally located with: Electric Fence 2.0.5

1 files changed, 7 insertions, 9 deletions

diff --git a/usr.sbin/ypserv/yp_server.c b/usr.sbin/ypserv/yp_server.c
index 3406ca0..35d65f0 100644
--- a/usr.sbin/ypserv/yp_server.c
+++ b/usr.sbin/ypserv/yp_server.c
@@ -45,7 +45,7 @@
 #include <rpc/rpc.h>
 
 #ifndef lint
-static const char rcsid[] = "$Id: yp_server.c,v 1.21 1997/04/10 14:12:51 wpaul Exp $";
+static const char rcsid[] = "$Id: yp_server.c,v 1.22 1997/04/28 14:18:38 wpaul Exp $";
 #endif /* not lint */
 
 int forked = 0;
@@ -159,21 +159,19 @@ ypproc_match_2_svc(ypreq_key *argp, struct svc_req *rqstp)
 #else
 	if (do_dns && result.stat != YP_TRUE && strstr(argp->map, "hosts")) {
 #endif
+		char			nbuf[YPMAXRECORD];
 	
 		/* NUL terminate! NUL terminate!! NUL TERMINATE!!! */
-		argp->key.keydat_val[argp->key.keydat_len] = '\0';
+		bcopy(argp->key.keydat_val, nbuf, argp->key.keydat_len);
+		nbuf[argp->key.keydat_len] = '\0';
 
 		if (debug)
-			yp_error("Doing DNS lookup of %.*s",
-			 	  argp->key.keydat_len,
-				  argp->key.keydat_val);
+			yp_error("Doing DNS lookup of %s", nbuf);
 
 		if (!strcmp(argp->map, "hosts.byname"))
-			result.stat = yp_async_lookup_name(rqstp,
-					(char *)argp->key.keydat_val);
+			result.stat = yp_async_lookup_name(rqstp, nbuf);
 		else if (!strcmp(argp->map, "hosts.byaddr"))
-			result.stat = yp_async_lookup_addr(rqstp,
-					(char *)argp->key.keydat_val);
+			result.stat = yp_async_lookup_addr(rqstp, nbuf);
 
 		if (result.stat == YP_TRUE)
 			return(NULL);