Created attachment 238276 [details] provoke a free() of an uninitialized pointer in kadmin kadmind_dispatch() says: kadm5_principal_ent_rec ent; ...; case kadm_create:{ ret = kadm5_ret_principal_ent(sp, &ent); ...; ret = krb5_ret_int32(sp, &mask); if(ret){ kadm5_free_principal_ent(contextp->context, &ent); kadm5_free_pincipal_ent(hand, princ) frees ent.mod_name (and in particular ent->mod_name->name.name_string.val): if(princ->mod_name) krb5_free_principal(context->context, princ->mod_name); However, certain client messages can cause princ->mod_name to be uninitialized (and contain a non-NULL garbage value). kadmind_dispatch() doesn't zero out ent. ret_principal_ent() in lib/kadm5/marshall.c is in charge of setting all fields from values sent by the client. For mod_name: if (mask & KADM5_MOD_NAME) { krb5_ret_int32(sp, &tmp); if(tmp) krb5_ret_principal(sp, &princ->mod_name); else princ->mod_name = NULL; } In this situation the mask bit is always set. krb5_ret_principal() can return an error for some client inputs -- but ret_principal_ent() ignores the error, and on such a code path mode_name is never set, and continues to hold the uninitialized value. One way this can happen is if the client message is too short, so that krb5_ret_principal() hits the end before it has read everything it needs. I've attached a demo; kdc and kadmind should be running, and the user must have kerberos tickets. # cc kadmind14a.c -lkrb5 # ./a.out A backtrace from kadmind. valgrind also sees the bad free(). #0 der_free_general_string (str=0x11ad9e1000000000) at /usr/rtm/symbsd/src/crypto/heimdal/lib/asn1/der_free.c:43 #1 0x000000070731254c in free_PrincipalName (data=0x67fafadac <recenti>) at asn1_krb5_asn1.c:961 #2 0x00000007073129ae in free_Principal (data=0x67fafadac <recenti>) at asn1_krb5_asn1.c:1122 #3 0x0000000704e6919c in krb5_free_principal (context=<optimized out>, p=0x67fafadac <recenti>) at /usr/rtm/symbsd/src/crypto/heimdal/lib/krb5/principal.c:84 #4 0x0000000701eaff68 in kadm5_free_principal_ent (server_handle=0x711ad9e10, princ=0x7009e6390) at /usr/rtm/symbsd/src/crypto/heimdal/lib/kadm5/free.c:66 #5 0x000000067faf6146 in kadmind_dispatch (kadm_handlep=0x711b1f4c0, initial=0, in=<optimized out>, out=0x7009e6568) at /usr/rtm/symbsd/src/crypto/heimdal/kadmin/server.c:122 #6 0x000000067faf5f16 in v5_loop (contextp=<optimized out>, ac=<optimized out>, initial=<optimized out>, kadm_handlep=<optimized out>, fd=<optimized out>) at /usr/rtm/symbsd/src/crypto/heimdal/kadmin/server.c:459 #7 0x000000067faf5dea in handle_v5 (contextp=0x711ad9e10, keytab=<optimized out>, fd=<optimized out>) at /usr/rtm/symbsd/src/crypto/heimdal/kadmin/server.c:551 #8 0x000000067faf5cf2 in kadmind_loop (contextp=0x711ad9e10,
Similarly, ret_principal_ent()'s call to krb5_ret_principal() needs to be checked, otherwise ret_principal_ent() can return "no error" even though princ->principal isn't valid.
I'll look at this.
Patched: bob# /usr/libexec/kadmind --debug 2022-11-25T15:27:19 CREATE: Invalid argument bob#
Created attachment 238347 [details] The patch. And, the patch.
Created attachment 238354 [details] The port patch This patch addresses security/heimdal. security/heimdal-devel is not affected. Our upstream has implemented a CHECK() macro.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=780f663df3a0fc30da5f0680c128087b1a05ea40 commit 780f663df3a0fc30da5f0680c128087b1a05ea40 Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-26 16:48:51 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-11-27 02:41:51 +0000 heimdal: Add missing kadm5 error checks Generally obtained from upstream 655c057769f56bd8cdb7d16e93f1e7a7cb260342. PR: 267944, 267972 Obtained from: Heimdal commit 655c057769f56bd8cdb7d16e93f1e7a7cb260342 MFC after: 3 days crypto/heimdal/lib/kadm5/marshall.c | 185 ++++++++++++++++++++---------------- 1 file changed, 105 insertions(+), 80 deletions(-)
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=e13150e28c93d9e74f419dcd17d2e2bad41715ad commit e13150e28c93d9e74f419dcd17d2e2bad41715ad Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-25 23:29:14 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-11-27 02:41:51 +0000 heimdal: Fix uninitialized pointer dereference krb5_ret_preincipal() returns a non-zero return code when a garbage principal is passed to it. Unfortunately ret_principal_ent() does not check the return code, with garbage pointing to what would have been the principal. This results in a segfault when free() is called. PR: 267944, 267972 Reported by: Robert Morris <rtm@lcs.mit.edu> MFC after: 3 days crypto/heimdal/lib/kadm5/marshall.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=0ae2fac599455cfef3348a52483d1d0c01ae1908 commit 0ae2fac599455cfef3348a52483d1d0c01ae1908 Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-26 16:48:51 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-12-01 14:25:10 +0000 heimdal: Add missing kadm5 error checks Generally obtained from upstream 655c057769f56bd8cdb7d16e93f1e7a7cb260342. PR: 267944, 267972 Obtained from: Heimdal commit 655c057769f56bd8cdb7d16e93f1e7a7cb260342 (cherry picked from commit 780f663df3a0fc30da5f0680c128087b1a05ea40) crypto/heimdal/lib/kadm5/marshall.c | 185 ++++++++++++++++++++---------------- 1 file changed, 105 insertions(+), 80 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=fed526b0af2bd709f8d187f0c7529d710d3a1bb0 commit fed526b0af2bd709f8d187f0c7529d710d3a1bb0 Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-25 23:29:14 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-12-01 14:25:10 +0000 heimdal: Fix uninitialized pointer dereference krb5_ret_preincipal() returns a non-zero return code when a garbage principal is passed to it. Unfortunately ret_principal_ent() does not check the return code, with garbage pointing to what would have been the principal. This results in a segfault when free() is called. PR: 267944, 267972 Reported by: Robert Morris <rtm@lcs.mit.edu> (cherry picked from commit e13150e28c93d9e74f419dcd17d2e2bad41715ad) crypto/heimdal/lib/kadm5/marshall.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=f3dc873719d4bdf7341c00097643843562e3114c commit f3dc873719d4bdf7341c00097643843562e3114c Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-26 16:48:51 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-12-01 14:25:53 +0000 heimdal: Add missing kadm5 error checks Generally obtained from upstream 655c057769f56bd8cdb7d16e93f1e7a7cb260342. PR: 267944, 267972 Obtained from: Heimdal commit 655c057769f56bd8cdb7d16e93f1e7a7cb260342 (cherry picked from commit 780f663df3a0fc30da5f0680c128087b1a05ea40) crypto/heimdal/lib/kadm5/marshall.c | 185 ++++++++++++++++++++---------------- 1 file changed, 105 insertions(+), 80 deletions(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=92586930433d215a606a9a50bfaa5358e4540acc commit 92586930433d215a606a9a50bfaa5358e4540acc Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-25 23:29:14 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-12-01 14:25:52 +0000 heimdal: Fix uninitialized pointer dereference krb5_ret_preincipal() returns a non-zero return code when a garbage principal is passed to it. Unfortunately ret_principal_ent() does not check the return code, with garbage pointing to what would have been the principal. This results in a segfault when free() is called. PR: 267944, 267972 Reported by: Robert Morris <rtm@lcs.mit.edu> (cherry picked from commit e13150e28c93d9e74f419dcd17d2e2bad41715ad) crypto/heimdal/lib/kadm5/marshall.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
fixed
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=f8c4316342857a4fa4a05c1cb6ab16992faddb69 commit f8c4316342857a4fa4a05c1cb6ab16992faddb69 Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-26 16:27:08 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2024-03-14 22:12:36 +0000 security/heimdal: Fix uninitialized pointer dereference krb5_ret_preincipal() returns a non-zero return code when a garbage principal is passed to it. Unfortunately ret_principal_ent() does not check the return code, with garbage pointing to what would have been the principal. This results in a segfault when free() is called. PR: 267944, 267972 Reported by: Robert Morris <rtm@lcs.mit.edu> MFH: 2024Q1 security/heimdal/Makefile | 2 +- security/heimdal/files/patch-lib_kadm5_marshall.c | 31 +++++++++++++++++++++-- 2 files changed, 30 insertions(+), 3 deletions(-)