MITKRB5-SA-2007-005: kadmind vulnerable to buffer overflow

[Russ Allbery]

Mike Friedman <mikef at ack.berkeley.edu> writes:

> My system does support vsnprintf(), so, I followed the above
> advice. Now, I'm faced with having to install 2007-05, which has the
> full 2007-02 patch as pre-requisite.

> Any suggestions as to the easiest way to proceed?  I'd like at present
> to avoid significant testing of a new release if it's likely to have
> some incompatibilities.  I'm not sure what the issues are between 1.5.3
> and 1.6.1 in this regard.

> If I had a version of 2007-05 that fit 1.4.2 with only the 'logger.c'
> portion of 2007-02 applied, that would, I suppose, be the best I could
> expect.  What are the chances of that?

The following patch against 1.4.4 compiles and appears to me to be safe
provided that your system supports vsnprintf, but I'd be happy to get an
additional review of that belief:

=== src/kadmin/server/server_stubs.c
==================================================================
--- src/kadmin/server/server_stubs.c	(revision 2543)
+++ src/kadmin/server/server_stubs.c	(local)
@@ -472,6 +472,8 @@
     OM_uint32			minor_stat;
     kadm5_server_handle_t	handle;
     restriction_t		*rp;
+    size_t			tlen1, tlen2, clen, slen;
+    char			*tdots1, *tdots2, *cdots, *sdots;
 
     xdr_free(xdr_generic_ret, &ret);
 
@@ -492,7 +494,14 @@
 	 ret.code = KADM5_BAD_PRINCIPAL;
 	 return &ret;
     }
-    sprintf(prime_arg, "%s to %s", prime_arg1, prime_arg2);
+    tlen1 = strlen(prime_arg1);
+    trunc_name(&tlen1, &tdots1);
+    tlen2 = strlen(prime_arg2);
+    trunc_name(&tlen2, &tdots2);
+    clen = client_name.length;
+    trunc_name(&clen, &cdots);
+    slen = service_name.length;
+    trunc_name(&slen, &sdots);
 
     ret.code = KADM5_OK;
     if (! CHANGEPW_SERVICE(rqstp)) {
@@ -510,17 +519,27 @@
     } else
 	 ret.code = KADM5_AUTH_INSUFFICIENT;
     if (ret.code != KADM5_OK) {
-	 krb5_klog_syslog(LOG_NOTICE, LOG_UNAUTH, "kadm5_rename_principal",
-		prime_arg, client_name.value, service_name.value,
-		inet_ntoa(rqstp->rq_xprt->xp_raddr.sin_addr));
+ 	 krb5_klog_syslog(LOG_NOTICE,
+ 			  "Unauthorized request: kadm5_rename_principal, "
+ 			  "%.*s%s to %.*s%s, "
+ 			  "client=%.*s%s, service=%.*s%s, addr=%s",
+ 			  tlen1, prime_arg1, tdots1,
+ 			  tlen2, prime_arg2, tdots2,
+ 			  clen, client_name.value, cdots,
+ 			  slen, service_name.value, sdots,
+ 			  inet_ntoa(rqstp->rq_xprt->xp_raddr.sin_addr));
     } else {
 	 ret.code = kadm5_rename_principal((void *)handle, arg->src,
 						arg->dest);
-	 krb5_klog_syslog(LOG_NOTICE, LOG_DONE, "kadm5_rename_principal",
-		prime_arg, ((ret.code == 0) ? "success" :
-			    error_message(ret.code)), 
-		client_name.value, service_name.value,
-		inet_ntoa(rqstp->rq_xprt->xp_raddr.sin_addr));
+ 	 krb5_klog_syslog(LOG_NOTICE,
+ 			  "Request: kadm5_rename_principal, "
+ 			  "%.*s%s to %.*s%s, %s, "
+ 			  "client=%.*s%s, service=%.*s%s, addr=%s",
+ 			  tlen1, prime_arg1, tdots1,
+ 			  tlen2, prime_arg2, tdots2, error_message(ret.code),
+ 			  clen, client_name.value, cdots,
+ 			  slen, service_name.value, sdots,
+ 			  inet_ntoa(rqstp->rq_xprt->xp_raddr.sin_addr));
     }
     free_server_handle(handle);
     free(prime_arg1);
=== src/kadmin/server/misc.c
==================================================================
--- src/kadmin/server/misc.c	(revision 2558)
+++ src/kadmin/server/misc.c	(local)
@@ -171,3 +171,12 @@
 
     return kadm5_free_principal_ent(handle->lhandle, &princ);
 }
+
+#define MAXPRINCLEN 125
+
+void
+trunc_name(size_t *len, char **dots)
+{
+    *dots = *len > MAXPRINCLEN ? "..." : "";
+    *len = *len > MAXPRINCLEN ? MAXPRINCLEN : *len;
+}
=== src/kadmin/server/misc.h
==================================================================
--- src/kadmin/server/misc.h	(revision 2558)
+++ src/kadmin/server/misc.h	(local)
@@ -45,3 +45,5 @@
 #ifdef SVC_GETARGS
 void  kadm_1(struct svc_req *, SVCXPRT *);
 #endif
+
+void trunc_name(size_t *len, char **dots);


-- 
Russ Allbery (rra at stanford.edu)             <http://www.eyrie.org/~eagle/>