Bug 20779
| Summary: | junk pointer error causes kpasswd to fail | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Robert Watson <rwatson> | ||||
| Component: | bin | Assignee: | assar <assar> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Only Me | ||||||
| Priority: | Normal | ||||||
| Version: | 4.1-STABLE | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
|
Description
Robert Watson
2000-08-22 15:20:01 UTC
Responsible Changed From-To: freebsd-bugs->assar Over to the maintainer. On Tue, Aug 22, 2000 at 10:11:18AM -0400, rwatson@freebsd.org wrote: > > fledge:~> passwd > realm WATSON.ORG > Old password for robert: > New Password for robert: > Verifying password - New Password for robert: > passwd in free(): warning: junk pointer, too high to make sense. > kpasswd: Couldn't access ticket file attempting to change password. > Password NOT changed. > Compiling passwd with -g and then running it with MALLOC_OPTIONS=A may be of some help here. -- Ruslan Ermilov Oracle Developer/DBA, ru@sunbay.com Sunbay Software AG, ru@FreeBSD.org FreeBSD committer, +380.652.512.251 Simferopol, Ukraine http://www.FreeBSD.org The Power To Serve http://www.oracle.com Enabling The Information Age On Tue, 22 Aug 2000, Ruslan Ermilov wrote: > On Tue, Aug 22, 2000 at 10:11:18AM -0400, rwatson@freebsd.org wrote: > > > > fledge:~> passwd > > realm WATSON.ORG > > Old password for robert: > > New Password for robert: > > Verifying password - New Password for robert: > > passwd in free(): warning: junk pointer, too high to make sense. > > kpasswd: Couldn't access ticket file attempting to change password. > > Password NOT changed. > > > Compiling passwd with -g and then running it with MALLOC_OPTIONS=A > may be of some help here. Getting some odd results here from attempting that. First off, setting that MALLOC_OPTIONS results in the following from most binaries: fledge:~> passwd passwd in malloc(): warning: unknown char in MALLOC_OPTIONS Second of all, passwd is setuid, so although it generates an Abort, we don't get a core dump: fledge:~> passwd passwd in malloc(): warning: unknown char in MALLOC_OPTIONS realm WATSON.ORG Old password for robert: New Password for robert: Verifying password - New Password for robert: passwd in free(): error: junk pointer, too high to make sense. Abort (this is different than with MALLOC_OPTIONS unset, however, so the A is doing something?) Removing the setuid bit on a local copy of passwd results in different behavior. First of all, it doesn't default to kerberos: fledge:~> ./passwd passwd in malloc(): warning: unknown char in MALLOC_OPTIONS passwd: error opening database: /etc/pwd.db.: Permission denied passwd: /etc/master.passwd: unchanged Even though it should do, as I only use Kerberos, which doesn't require privilege for the password to be changed. However, forcing the use of Kerberos using the ``-r'' flag results in the password change working (?): fledge:~> ./passwd -r WATSON.ORG passwd in malloc(): warning: unknown char in MALLOC_OPTIONS realm WATSON.ORG Old password for robert@WATSON.ORG: New Password for robert@WATSON.ORG: Verifying password - New Password for robert@WATSON.ORG: Password changed. Bizarre, huh. :-) In any case, the work-around for the bug appears to be to force the realm name on a non-setuid copy of the binary. With the setuid copy, regardless of forcing the realm name, I still get an Abort: fledge:~> passwd -r WATSON.ORG passwd in malloc(): warning: unknown char in MALLOC_OPTIONS realm WATSON.ORG Old password for robert@WATSON.ORG: New Password for robert@WATSON.ORG: Verifying password - New Password for robert@WATSON.ORG: passwd in free(): error: junk pointer, too high to make sense. Abort Robert N M Watson robert@fledge.watson.org http://www.watson.org/~robert/ PGP key fingerprint: AF B5 5F FF A6 4A 79 37 ED 5F 55 E9 58 04 6A B1 TIS Labs at Network Associates, Safeport Network Services On Tue, Aug 22, 2000 at 01:36:00PM -0400, Robert Watson wrote: > On Tue, 22 Aug 2000, Ruslan Ermilov wrote: > [...] > > Compiling passwd with -g and then running it with MALLOC_OPTIONS=A > > may be of some help here. > > Getting some odd results here from attempting that. First off, setting > that MALLOC_OPTIONS results in the following from most binaries: > > fledge:~> passwd > passwd in malloc(): warning: unknown char in MALLOC_OPTIONS > Make sure you did not put any extraneous characters, you can check with `echo x${MALLOC_OPTIONS}x'. > Second of all, passwd is setuid, so although it generates an Abort, we > don't get a core dump: > > fledge:~> passwd > passwd in malloc(): warning: unknown char in MALLOC_OPTIONS > realm WATSON.ORG > Old password for robert: > New Password for robert: > Verifying password - New Password for robert: > passwd in free(): error: junk pointer, too high to make sense. > Abort > Did you try `sysctl -w kern.sugid_coredump=1'? > (this is different than with MALLOC_OPTIONS unset, however, so the A is > doing something?) > > Removing the setuid bit on a local copy of passwd results in different > behavior. First of all, it doesn't default to kerberos: > > fledge:~> ./passwd > passwd in malloc(): warning: unknown char in MALLOC_OPTIONS > passwd: error opening database: /etc/pwd.db.: Permission denied > passwd: /etc/master.passwd: unchanged > > Even though it should do, as I only use Kerberos, which doesn't require > privilege for the password to be changed. However, forcing the use of > Kerberos using the ``-r'' flag results in the password change working (?): > > fledge:~> ./passwd -r WATSON.ORG > passwd in malloc(): warning: unknown char in MALLOC_OPTIONS > realm WATSON.ORG > Old password for robert@WATSON.ORG: > New Password for robert@WATSON.ORG: > Verifying password - New Password for robert@WATSON.ORG: > Password changed. > > Bizarre, huh. :-) In any case, the work-around for the bug appears to be > to force the realm name on a non-setuid copy of the binary. With the > setuid copy, regardless of forcing the realm name, I still get an Abort: > > fledge:~> passwd -r WATSON.ORG > passwd in malloc(): warning: unknown char in MALLOC_OPTIONS > realm WATSON.ORG > Old password for robert@WATSON.ORG: > New Password for robert@WATSON.ORG: > Verifying password - New Password for robert@WATSON.ORG: > passwd in free(): error: junk pointer, too high to make sense. > Abort > > Robert N M Watson -- Ruslan On Wed, 23 Aug 2000, Ruslan Ermilov wrote: > > Getting some odd results here from attempting that. First off, setting > > that MALLOC_OPTIONS results in the following from most binaries: > > > > fledge:~> passwd > > passwd in malloc(): warning: unknown char in MALLOC_OPTIONS > > > Make sure you did not put any extraneous characters, you can check with > `echo x${MALLOC_OPTIONS}x'. That seems to have been the problem. I normally use tcsh, but switching to sh and setting MALLOC_OPTIONS seemed to work a lot better. > > Second of all, passwd is setuid, so although it generates an Abort, we > > don't get a core dump: > ... > Did you try `sysctl -w kern.sugid_coredump=1'? Ok, here we go: (gdb) where #0 0x28185d38 in kill () from /usr/lib/libc.so.4 #1 0x281c2c6a in abort () from /usr/lib/libc.so.4 #2 0x281c1781 in isatty () from /usr/lib/libc.so.4 #3 0x281c17b7 in isatty () from /usr/lib/libc.so.4 #4 0x281c26be in isatty () from /usr/lib/libc.so.4 #5 0x281c2915 in free () from /usr/lib/libc.so.4 #6 0x2806c1ef in kadm_change_pw_plain () from /usr/lib/libkadm.so.3 #7 0x2806c259 in kadm_change_pw () from /usr/lib/libkadm.so.3 #8 0x804d377 in krb_passwd (uname=0x0, iflag=0x0, rflag=0x0, uflag=0x0) at /data/fbsd-stable/src/usr.bin/passwd/../../crypto/kerberosIV/kadmin/kpasswd_standalone.c:143 #9 0x804a7d8 in main (argc=1, argv=0xbfbff720) at /data/fbsd-stable/src/usr.bin/passwd/passwd.c:220 #10 0x804a0c5 in _start () Unfortunately, I don't have libc or libkadm built with debugging symbols at this point. However, here's the information on kpasswd_standalone.c:143: (gdb) list 138 get_pw_new_key(new_key, name, inst, realm, realm_given); 139 140 if ((status = kadm_init_link("changepw", KRB_MASTER, realm)) 141 != KADM_SUCCESS) 142 com_err("kpasswd", status, "while initializing"); 143 else if ((status = kadm_change_pw(new_key)) != KADM_SUCCESS) 144 com_err("kpasswd", status, " attempting to change password."); 145 146 if (status != KADM_SUCCESS) 147 fprintf(stderr,"Password NOT changed.\n"); (gdb) inspect new_key $1 = "XXXXXXXXXXX" (gdb) inspect name $2 = "robert", '\000' <repeats 33 times> (gdb) inspect inst $3 = '\000' <repeats 39 times> (gdb) inspect realm $4 = "WATSON.ORG", '\000' <repeats 29 times> (gdb) inspect realm_given $5 = 0 (gdb) inspect status $6 = 0 Robert N M Watson robert@fledge.watson.org http://www.watson.org/~robert/ PGP key fingerprint: AF B5 5F FF A6 4A 79 37 ED 5F 55 E9 58 04 6A B1 TIS Labs at Network Associates, Safeport Network Services This problem still plagues 4.x and 5.x. I dug around a bit last night but haven't found the problem yet. However, as previously reported, removing the suid bit causes the fault to stop, but will cause problems with local passwords. Until the cause is found, I have been using the following patch to drop root privs when kerberos is used (repeat after me, dropping root privs when not needed is good). -- Chris D. Faulhaber - jedgar@fxp.org - jedgar@FreeBSD.org -------------------------------------------------------- FreeBSD: The Power To Serve - http://www.FreeBSD.org Index: passwd.c =================================================================== RCS file: /home/ncvs/src/usr.bin/passwd/passwd.c,v retrieving revision 1.16 diff -u -r1.16 passwd.c --- passwd.c 1999/08/28 01:04:52 1.16 +++ passwd.c 2000/12/28 16:02:35 @@ -217,6 +217,7 @@ if (k && strstr(k, "kerberos")) if(krb_get_lrealm(realm, 0) == KSUCCESS) { fprintf(stderr, "realm %s\n", realm); + setuid(getuid()); exit(krb_passwd(argv[0], iflag, rflag, uflag)); } #endif Chris's patch is needed, otherwise libkrb is not confortable reading
your ticket file, but the bad freeing that you were seeing seems to be
caused by bad dynamic memory allocation. I think the appended patch
fixes that.
/assar
Index: kadm_cli_wrap.c
===================================================================
RCS file: /afs/pdc.kth.se/src/packages/kth-krb/SourceRepository/krb4/lib/kadm/kadm_cli_wrap.c,v
retrieving revision 1.29
diff -u -w -u -w -r1.29 kadm_cli_wrap.c
--- kadm_cli_wrap.c 2000/07/08 12:12:00 1.29
+++ kadm_cli_wrap.c 2001/02/21 05:02:55
@@ -165,6 +165,7 @@
int retval;
char tmp[4];
+ *ret_dat = NULL;
dlen = (u_int16_t) dat_len;
if (dat_len != (int)dlen)
@@ -193,6 +194,8 @@
if ((retval = krb_net_read(client_parm.admin_fd, *ret_dat,
dlen) != dlen)) {
+ free(*ret_dat);
+ *ret_dat = NULL;
if (retval < 0)
return(errno); /* XXX */
else
@@ -307,7 +310,7 @@
clear_secrets();
return retdat;
}
-#define RET_N_FREE2(r) {free(*ret_dat); clear_secrets(); return(r);}
+#define RET_N_FREE2(r) {free(*ret_dat); *ret_dat = NULL; clear_secrets(); return(r);}
/* first see if it's a YOULOUSE */
if ((*ret_siz >= KADM_VERSIZE) &&
I wrote:
> Chris's patch is needed, otherwise libkrb is not confortable reading
> your ticket file, but the bad freeing that you were seeing seems to be
> caused by bad dynamic memory allocation. I think the appended patch
> fixes that.
Now that I've finally had a chance to test this, I think this is the
right patch. Could you look at and I'll commit it if you think it's
ok.
/assar
Index: usr.bin/passwd/passwd.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/passwd/passwd.c,v
retrieving revision 1.16
diff -u -w -r1.16 passwd.c
--- usr.bin/passwd/passwd.c 1999/08/28 01:04:52 1.16
+++ usr.bin/passwd/passwd.c 2001/02/21 12:54:03
@@ -216,6 +216,7 @@
k = auth_getval("auth_list");
if (k && strstr(k, "kerberos"))
if(krb_get_lrealm(realm, 0) == KSUCCESS) {
+ setuid(getuid());
fprintf(stderr, "realm %s\n", realm);
exit(krb_passwd(argv[0], iflag, rflag, uflag));
}
Index: crypto/kerberosIV/lib/kadm/kadm_cli_wrap.c
===================================================================
RCS file: /home/ncvs/src/crypto/kerberosIV/lib/kadm/kadm_cli_wrap.c,v
retrieving revision 1.1.1.3
diff -u -w -r1.1.1.3 kadm_cli_wrap.c
--- crypto/kerberosIV/lib/kadm/kadm_cli_wrap.c 2000/01/09 08:28:09 1.1.1.3
+++ crypto/kerberosIV/lib/kadm/kadm_cli_wrap.c 2001/02/21 12:54:06
@@ -193,6 +193,8 @@
if ((retval = krb_net_read(client_parm.admin_fd, *ret_dat,
dlen) != dlen)) {
+ free(*ret_dat);
+ *ret_dat = NULL;
if (retval < 0)
return(errno); /* XXX */
else
@@ -238,6 +240,9 @@
int tmp;
void *tmp_ptr;
+ *ret_dat = NULL;
+ *ret_siz = 0;
+
act_st = malloc(KADM_VERSIZE); /* verstr stored first */
if (act_st == NULL) {
clear_secrets ();
@@ -309,7 +314,7 @@
clear_secrets();
return retdat;
}
-#define RET_N_FREE2(r) {free(*ret_dat); clear_secrets(); return(r);}
+#define RET_N_FREE2(r) {free(*ret_dat); *ret_dat = NULL; clear_secrets(); return(r);}
/* first see if it's a YOULOUSE */
if ((*ret_siz >= KADM_VERSIZE) &&
@@ -410,8 +415,6 @@
msg[0]=0;
*pw_msg=msg;
}
-
- if (ret_st)
free(ret_st);
kadm_cli_disconn();
@@ -481,8 +484,8 @@
/* ret_st has vals */
if (stream_to_vals(ret_st, vals, ret_sz) < 0)
retc = KADM_LENGTH_ERROR;
- free(ret_st);
}
+ free(ret_st);
kadm_cli_disconn();
return(retc);
}
@@ -539,8 +542,8 @@
/* ret_st has vals */
if (stream_to_vals(ret_st, vals2, ret_sz) < 0)
retc = KADM_LENGTH_ERROR;
- free(ret_st);
}
+ free(ret_st);
kadm_cli_disconn();
return(retc);
}
@@ -571,6 +574,7 @@
}
retc = kadm_cli_send(st2, st_len + 1, &ret_st, &ret_sz);
free(st2);
+ free(ret_st);
kadm_cli_disconn();
return(retc);
}
@@ -618,8 +622,8 @@
/* ret_st has vals */
if (stream_to_vals(ret_st, vals, ret_sz) < 0)
retc = KADM_LENGTH_ERROR;
- free(ret_st);
}
+ free(ret_st);
kadm_cli_disconn();
return(retc);
}
State Changed From-To: open->feedback proposed patch sent to submitter Hmm. Now I get the following: (gdb) where #0 0x28075d4f in strnlen () from /usr/lib/libkrb.so.3 #1 0x2806ea1d in stv_string () from /usr/lib/libkadm.so.3 #2 0x2806e2c5 in kadm_change_pw_plain () from /usr/lib/libkadm.so.3 #3 0x2806e34d in kadm_change_pw () from /usr/lib/libkadm.so.3 #4 0x804d4cb in krb_get_pw_in_tkt () #5 0x804a8e5 in krb_get_pw_in_tkt () #6 0x804a16d in krb_get_pw_in_tkt () I won't have a chance to stick this under a debugger properly for a few days, but hopefully this trace is useful. Robert N M Watson FreeBSD Core Team, TrustedBSD Project robert@fledge.watson.org NAI Labs, Safeport Network Services On 21 Feb 2001, Assar Westerlund wrote: > I wrote: > > Chris's patch is needed, otherwise libkrb is not confortable reading > > your ticket file, but the bad freeing that you were seeing seems to be > > caused by bad dynamic memory allocation. I think the appended patch > > fixes that. > > Now that I've finally had a chance to test this, I think this is the > right patch. Could you look at and I'll commit it if you think it's > ok. > > /assar > > Index: usr.bin/passwd/passwd.c > =================================================================== > RCS file: /home/ncvs/src/usr.bin/passwd/passwd.c,v > retrieving revision 1.16 > diff -u -w -r1.16 passwd.c > --- usr.bin/passwd/passwd.c 1999/08/28 01:04:52 1.16 > +++ usr.bin/passwd/passwd.c 2001/02/21 12:54:03 > @@ -216,6 +216,7 @@ > k = auth_getval("auth_list"); > if (k && strstr(k, "kerberos")) > if(krb_get_lrealm(realm, 0) == KSUCCESS) { > + setuid(getuid()); > fprintf(stderr, "realm %s\n", realm); > exit(krb_passwd(argv[0], iflag, rflag, uflag)); > } > Index: crypto/kerberosIV/lib/kadm/kadm_cli_wrap.c > =================================================================== > RCS file: /home/ncvs/src/crypto/kerberosIV/lib/kadm/kadm_cli_wrap.c,v > retrieving revision 1.1.1.3 > diff -u -w -r1.1.1.3 kadm_cli_wrap.c > --- crypto/kerberosIV/lib/kadm/kadm_cli_wrap.c 2000/01/09 08:28:09 1.1.1.3 > +++ crypto/kerberosIV/lib/kadm/kadm_cli_wrap.c 2001/02/21 12:54:06 > @@ -193,6 +193,8 @@ > > if ((retval = krb_net_read(client_parm.admin_fd, *ret_dat, > dlen) != dlen)) { > + free(*ret_dat); > + *ret_dat = NULL; > if (retval < 0) > return(errno); /* XXX */ > else > @@ -238,6 +240,9 @@ > int tmp; > void *tmp_ptr; > > + *ret_dat = NULL; > + *ret_siz = 0; > + > act_st = malloc(KADM_VERSIZE); /* verstr stored first */ > if (act_st == NULL) { > clear_secrets (); > @@ -309,7 +314,7 @@ > clear_secrets(); > return retdat; > } > -#define RET_N_FREE2(r) {free(*ret_dat); clear_secrets(); return(r);} > +#define RET_N_FREE2(r) {free(*ret_dat); *ret_dat = NULL; clear_secrets(); return(r);} > > /* first see if it's a YOULOUSE */ > if ((*ret_siz >= KADM_VERSIZE) && > @@ -410,8 +415,6 @@ > msg[0]=0; > *pw_msg=msg; > } > - > - if (ret_st) > free(ret_st); > > kadm_cli_disconn(); > @@ -481,8 +484,8 @@ > /* ret_st has vals */ > if (stream_to_vals(ret_st, vals, ret_sz) < 0) > retc = KADM_LENGTH_ERROR; > - free(ret_st); > } > + free(ret_st); > kadm_cli_disconn(); > return(retc); > } > @@ -539,8 +542,8 @@ > /* ret_st has vals */ > if (stream_to_vals(ret_st, vals2, ret_sz) < 0) > retc = KADM_LENGTH_ERROR; > - free(ret_st); > } > + free(ret_st); > kadm_cli_disconn(); > return(retc); > } > @@ -571,6 +574,7 @@ > } > retc = kadm_cli_send(st2, st_len + 1, &ret_st, &ret_sz); > free(st2); > + free(ret_st); > kadm_cli_disconn(); > return(retc); > } > @@ -618,8 +622,8 @@ > /* ret_st has vals */ > if (stream_to_vals(ret_st, vals, ret_sz) < 0) > retc = KADM_LENGTH_ERROR; > - free(ret_st); > } > + free(ret_st); > kadm_cli_disconn(); > return(retc); > } > Better patch (that works for me applied to RELENG-4) attached. I would like to commit this to current and ask jkh for pulling it up to RELENG-4. /assar Sorry I haven't had a chance to review this -- life is pretty busy right now due to conferences/proposals/et al. I'll hopefully get a chance to test it out later this week. Saw the commit, which needless to say makes it easier to test :-). I've had at least one independent confirmation, btw, that the commit fixed this for at least two sites using 4.2-STABLE/4.3-BETA, so it seems right at face value. Robert N M Watson FreeBSD Core Team, TrustedBSD Project robert@fledge.watson.org NAI Labs, Safeport Network Services On 8 Mar 2001, Assar Westerlund wrote: > Robert Watson <rwatson@freebsd.org> writes: > > Hmm. Now I get the following: > > > > (gdb) where > > #0 0x28075d4f in strnlen () from /usr/lib/libkrb.so.3 > > #1 0x2806ea1d in stv_string () from /usr/lib/libkadm.so.3 > > #2 0x2806e2c5 in kadm_change_pw_plain () from /usr/lib/libkadm.so.3 > > #3 0x2806e34d in kadm_change_pw () from /usr/lib/libkadm.so.3 > > #4 0x804d4cb in krb_get_pw_in_tkt () > > #5 0x804a8e5 in krb_get_pw_in_tkt () > > #6 0x804a16d in krb_get_pw_in_tkt () > > > > I won't have a chance to stick this under a debugger properly for a few > > days, but hopefully this trace is useful. > > Better patch (that works for me applied to RELENG-4) attached. I > would like to commit this to current and ask jkh for pulling it up to > RELENG-4. > > /assar > > State Changed From-To: feedback->closed Kerberos IV has been deleted. |