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

[Lee Hinman]

Peter,

Just a little suggestion on your patch.  Calling error_message 
(ret.code) when ret.code == 0 may cause your output to be something  
like "Unknown error: 0".  It will depend on what your libc does when  
you call sterror(0).  Previously it would print out "success".  The  
change below restores that behavior.

--
Lee

On Jun 27, 2007, at 11:06 AM, krbdev-request at mit.edu wrote:

>
> Message: 1
> Date: Wed, 27 Jun 2007 06:59:57 -0700
> From: Russ Allbery <rra at stanford.edu>
> Subject: Re: MITKRB5-SA-2007-005: kadmind vulnerable to buffer
> 	overflow
> To: Peter Bosanko <pb10 at cornell.edu>
> Cc: krbdev at mit.edu
> Message-ID: <87bqf1sdk2.fsf at windlord.stanford.edu>
> Content-Type: text/plain; charset=us-ascii
>
> Peter Bosanko <pb10 at cornell.edu> writes:
>
>> Does the vulnerability MITKRB5-SA-2007-005 apply to Kerberos  
>> 1.3.6?  If
>> so, has anyone got a patch? :-)
>
> It looked like it applied to me.  A backport of the patch to 1.4.4  
> also
> applied to 1.3.6 with a minor bit of fuzz.  Here's a patch that  
> doesn't
> require that the full 2007-002 patch be applied, although please  
> note that
> this is "works for me" and haven't been thoroughly tested, and in
> particular haven't been thoroughly tested against an exploit.  I'd  
> welcome
> any and all feedback about any flaws in these backports.  It also  
> assumes
> that your system has a working vsnprintf and that you're building  
> it with
> HAVE_VSNPRINTF enabled and have the patch from -002 applied to use
> vsnprintf.
>
> === 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),

			    tlen2, prime_arg2, tdots2,
                             ((ret.code == 0) ? "success" :  
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/>
>
>
> ------------------------------
>
> _______________________________________________
> krbdev mailing list
> krbdev at mit.edu
> https://mailman.mit.edu/mailman/listinfo/krbdev
>
>