Ticket History #1140: kadmind dumps core when it receives bad data from client

From mrl@isc.upenn.edu Thu Jul 25 16:27:43 2002
Received: from fort-point-station.mit.edu (FORT-POINT-STATION.MIT.EDU [18.7.7.76])
by rt-11.mit.edu (8.9.3/8.9.3) with ESMTP id QAA17322
for <bugs@RT-11.mit.edu>; Thu, 25 Jul 2002 16:27:43 -0400 (EDT)
Received: from falstaff.isc-net.upenn.edu (FALSTAFF.ISC-NET.UPENN.EDU [130.91.72.121])
by fort-point-station.mit.edu (8.9.2/8.9.2) with ESMTP id QAA15520
for <krb5-bugs@mit.edu>; Thu, 25 Jul 2002 16:27:42 -0400 (EDT)
Received: from isc.upenn.edu (gnaghi.isc-net.upenn.edu [165.123.50.51])
by falstaff.isc-net.upenn.edu (8.12.3/8.12.3) with ESMTP id g6PKRgK4002486;
Thu, 25 Jul 2002 16:27:42 -0400 (EDT)
Message-Id: <200207252027.g6PKRgK4002486@falstaff.isc-net.upenn.edu>
Date: Thu, 25 Jul 2002 16:27:42 -0400
From: mrl@isc.upenn.edu
Reply-To: mrl@isc.upenn.edu
To: krb5-bugs@mit.edu
Cc: mrl@isc.upenn.edu
Subject: Patch for reproducible kadmind crash
X-Send-Pr-Version: 3.99

University of Pennsylvania
System: SunOS gnaghi.isc-net.upenn.edu 5.8 Generic_108528-12 sun4u sparc SUNW,Ultra-5_10
Architecture: sun4

Bad data in client requests can reliably cause kadmind to crash in
krb5_unparse_name_ext(), where it tries to dereference a NULL pointer.

The patch below prevents these crashes by checking for a NULL principal
argument in krb5_unparse_name_ext(), and checking the return values from
several calls to krb5_unparse_name() in kadmin/server/server_stubs.c.

Notes on the patch:

- I've reproduced this crash only in create_principal_1(), but the
patch also fixes the other calls to krb5_unparse_name() in
server_stubs.c that don't check its return value.

- I chose KADM5_BAD_PRINCIPAL as the error code to return from the
kadmin server functions when krb5_unparse_name() fails, although
doc/kadm5/api-funcspec.tex doesn't document this as an expected return
code for those functions.

- Currently any bad attempts that fail due to krb5_unparse_name() errors
aren't logged. It might be desirable to log them.

I found this bug when someone made a mistake while experimenting with the
Authen::Krb5::Admin module for Perl, available from
http://www.cpan.org/modules/by-module/Authen/Authen-Krb5-Admin-0.03.tar.gz

I haven't developed C code to exercise the bug, but here's a minimal Perl
script that reliably crashes kadmind due to incorrect usage of the
Authen::Krb5::Admin module:

#!/usr/local/bin/perl

use Authen::Krb5;
Authen::Krb5::init_context();
Authen::Krb5::init_ets();

print "Principal: ";
chomp($principal = scalar(<STDIN>));
print "Password: ";
chomp($password = scalar(<STDIN>));

use Authen::Krb5::Admin;
$kadm5 = Authen::Krb5::Admin->init_with_password($principal, $password)
or die Authen::Krb5::Admin::error;

$x = Authen::Krb5::Admin::Principal::new('') or die Authen::Krb5::Admin::error;
$rc = $kadm5->create_principal($x, '') or die Authen::Krb5::Admin::error;

*** src/lib/krb5/krb/unparse.c Thu Jul 25 13:54:34 2002
--- src/lib/krb5/krb/unparse.c Thu Jul 25 09:41:34 2002
***************
*** 70,75 ****
--- 70,78 ----
krb5_int32 nelem;
register int totalsize = 0;

+ if (!principal)
+ return KRB5_PARSE_MALFORMED;
+
cp = krb5_princ_realm(context, principal)->data;
length = krb5_princ_realm(context, principal)->length;
totalsize += length;
*** src/kadmin/server/server_stubs.c Thu Jul 25 13:54:34 2002
--- src/kadmin/server/server_stubs.c Thu Jul 25 13:28:07 2002
***************
*** 257,263 ****
ret.code = KADM5_FAILURE;
return &ret;
}
! krb5_unparse_name(handle->context, arg->rec.principal, &prime_arg);

if (CHANGEPW_SERVICE(rqstp)
|| !acl_check(handle->context, rqstp->rq_clntcred, ACL_ADD,
--- 257,266 ----
ret.code = KADM5_FAILURE;
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->rec.principal, &prime_arg)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }

if (CHANGEPW_SERVICE(rqstp)
|| !acl_check(handle->context, rqstp->rq_clntcred, ACL_ADD,
***************
*** 311,317 ****
ret.code = KADM5_FAILURE;
return &ret;
}
! krb5_unparse_name(handle->context, arg->rec.principal, &prime_arg);

if (CHANGEPW_SERVICE(rqstp)
|| !acl_check(handle->context, rqstp->rq_clntcred, ACL_ADD,
--- 314,323 ----
ret.code = KADM5_FAILURE;
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->rec.principal, &prime_arg)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }

if (CHANGEPW_SERVICE(rqstp)
|| !acl_check(handle->context, rqstp->rq_clntcred, ACL_ADD,
***************
*** 367,373 ****
ret.code = KADM5_FAILURE;
return &ret;
}
! krb5_unparse_name(handle->context, arg->princ, &prime_arg);

if (CHANGEPW_SERVICE(rqstp)
|| !acl_check(handle->context, rqstp->rq_clntcred, ACL_DELETE,
--- 373,382 ----
ret.code = KADM5_FAILURE;
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->princ, &prime_arg)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }

if (CHANGEPW_SERVICE(rqstp)
|| !acl_check(handle->context, rqstp->rq_clntcred, ACL_DELETE,
***************
*** 415,421 ****
ret.code = KADM5_FAILURE;
return &ret;
}
! krb5_unparse_name(handle->context, arg->rec.principal, &prime_arg);

if (CHANGEPW_SERVICE(rqstp)
|| !acl_check(handle->context, rqstp->rq_clntcred, ACL_MODIFY,
--- 424,433 ----
ret.code = KADM5_FAILURE;
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->rec.principal, &prime_arg)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }

if (CHANGEPW_SERVICE(rqstp)
|| !acl_check(handle->context, rqstp->rq_clntcred, ACL_MODIFY,
***************
*** 469,476 ****
ret.code = KADM5_FAILURE;
return &ret;
}
! krb5_unparse_name(handle->context, arg->src, &prime_arg1);
! krb5_unparse_name(handle->context, arg->dest, &prime_arg2);
sprintf(prime_arg, "%s to %s", prime_arg1, prime_arg2);

ret.code = KADM5_OK;
--- 481,491 ----
ret.code = KADM5_FAILURE;
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->src, &prime_arg1) ||
! krb5_unparse_name(handle->context, arg->dest, &prime_arg2)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }
sprintf(prime_arg, "%s to %s", prime_arg1, prime_arg2);

ret.code = KADM5_OK;
***************
*** 539,545 ****
ret.code = KADM5_FAILURE;
return &ret;
}
! krb5_unparse_name(handle->context, arg->princ, &prime_arg);

if (! cmp_gss_krb5_name(handle, rqstp->rq_clntcred, arg->princ) &&
(CHANGEPW_SERVICE(rqstp) || !acl_check(handle->context,
--- 554,563 ----
ret.code = KADM5_FAILURE;
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->princ, &prime_arg)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }

if (! cmp_gss_krb5_name(handle, rqstp->rq_clntcred, arg->princ) &&
(CHANGEPW_SERVICE(rqstp) || !acl_check(handle->context,
***************
*** 659,665 ****
ret.code = KADM5_FAILURE;
return &ret;
}
! krb5_unparse_name(handle->context, arg->princ, &prime_arg);

if (cmp_gss_krb5_name(handle, rqstp->rq_clntcred, arg->princ)) {
ret.code = chpass_principal_wrapper((void *)handle, arg->princ,
--- 677,686 ----
ret.code = KADM5_FAILURE;
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->princ, &prime_arg)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }

if (cmp_gss_krb5_name(handle, rqstp->rq_clntcred, arg->princ)) {
ret.code = chpass_principal_wrapper((void *)handle, arg->princ,
***************
*** 717,723 ****
ret.code = KADM5_FAILURE;
return &ret;
}
! krb5_unparse_name(handle->context, arg->princ, &prime_arg);

if (cmp_gss_krb5_name(handle, rqstp->rq_clntcred, arg->princ)) {
ret.code = chpass_principal_wrapper((void *)handle, arg->princ,
--- 738,747 ----
ret.code = KADM5_FAILURE;
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->princ, &prime_arg)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }

if (cmp_gss_krb5_name(handle, rqstp->rq_clntcred, arg->princ)) {
ret.code = chpass_principal_wrapper((void *)handle, arg->princ,
***************
*** 778,784 ****
ret.code = KADM5_FAILURE;
return &ret;
}
! krb5_unparse_name(handle->context, arg->princ, &prime_arg);

if (!(CHANGEPW_SERVICE(rqstp)) &&
acl_check(handle->context, rqstp->rq_clntcred,
--- 802,811 ----
ret.code = KADM5_FAILURE;
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->princ, &prime_arg)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }

if (!(CHANGEPW_SERVICE(rqstp)) &&
acl_check(handle->context, rqstp->rq_clntcred,
***************
*** 833,839 ****
ret.code = KADM5_FAILURE;
return &ret;
}
! krb5_unparse_name(handle->context, arg->princ, &prime_arg);

if (!(CHANGEPW_SERVICE(rqstp)) &&
acl_check(handle->context, rqstp->rq_clntcred,
--- 860,869 ----
ret.code = KADM5_FAILURE;
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->princ, &prime_arg)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }

if (!(CHANGEPW_SERVICE(rqstp)) &&
acl_check(handle->context, rqstp->rq_clntcred,
***************
*** 888,894 ****
ret.code = KADM5_FAILURE;
return &ret;
}
! krb5_unparse_name(handle->context, arg->princ, &prime_arg);

if (!(CHANGEPW_SERVICE(rqstp)) &&
acl_check(handle->context, rqstp->rq_clntcred,
--- 918,927 ----
ret.code = KADM5_FAILURE;
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->princ, &prime_arg)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }

if (!(CHANGEPW_SERVICE(rqstp)) &&
acl_check(handle->context, rqstp->rq_clntcred,
***************
*** 952,958 ****
free_server_handle(handle);
return &ret;
}
! krb5_unparse_name(handle->context, arg->princ, &prime_arg);

if (cmp_gss_krb5_name(handle, rqstp->rq_clntcred, arg->princ)) {
ret.code = randkey_principal_wrapper((void *)handle,
--- 985,994 ----
free_server_handle(handle);
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->princ, &prime_arg)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }

if (cmp_gss_krb5_name(handle, rqstp->rq_clntcred, arg->princ)) {
ret.code = randkey_principal_wrapper((void *)handle,
***************
*** 1025,1031 ****
free_server_handle(handle);
return &ret;
}
! krb5_unparse_name(handle->context, arg->princ, &prime_arg);

if (cmp_gss_krb5_name(handle, rqstp->rq_clntcred, arg->princ)) {
ret.code = randkey_principal_wrapper((void *)handle,
--- 1061,1070 ----
free_server_handle(handle);
return &ret;
}
! if (krb5_unparse_name(handle->context, arg->princ, &prime_arg)) {
! ret.code = KADM5_BAD_PRINCIPAL;
! return &ret;
! }

if (cmp_gss_krb5_name(handle, rqstp->rq_clntcred, arg->princ)) {
ret.code = randkey_principal_wrapper((void *)handle,

Responsible-Changed-From-To: krb5-unassigned->tlyu
Responsible-Changed-By: tlyu
Responsible-Changed-When: Thu Aug 1 21:04:10 2002
Responsible-Changed-Why:

assigned

State-Changed-From-To: open-feedback
State-Changed-By: tlyu
State-Changed-When: Thu Aug 1 21:04:30 2002
State-Changed-Why:

patch applied


From: Tom Yu <tlyu@MIT.EDU>
To: mrl@isc.upenn.edu
Cc: krb5-bugs@MIT.EDU, mrl@isc.upenn.edu
Subject: Re: krb5-admin/1140: kadmind dumps core when it receives bad data from client
Date: Thu, 1 Aug 2002 21:05:09 -0400 (EDT)

Thanks for reporting this bug; your patch has been applied and should
be in the next release.

---Tom