Created attachment 238312 [details] crash kadmind with a short kadm_modify message If a client kadm_modify message ends unexpectedly early during KADM5_TL_DATA, krb5_ret_data() can return (due to error) before setting data->size of data->length. But the call from kadm5_ret_tl_data() doesn't check for an error, and the surrounding call from ret_principal_ent() for KADM5_TL_DATA doesn't check for an error either. So list elements in ent.tl_data (princ->tl_data) may contain uninitialized junk. I've attached a demo. It expects to be run with tickets. Maybe some previous bugs have to be fixed in order for kadmind to get as far as this one. # cc kadmind16a.c -lkrb5 # ./a.out A backtrace from kadmin: #0 memset (xdst=0x17e4ffb480, c=0, len=18446744073709541600) #1 0x00000017d6b6bff2 in kadm5_free_principal_ent ( server_handle=<optimized out>, princ=0x17d5c2f420) at /usr/rtm/symbsd/src/crypto/heimdal/lib/kadm5/free.c:73 #2 0x0000001754e3c340 in kadmind_dispatch (kadm_handlep=0x17e4fd44c0, initial=0, in=<optimized out>, out=0x17d5c2f5f8) at /usr/rtm/symbsd/src/crypto/heimdal/kadmin/server.c:161 #3 0x0000001754e3bf9e 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 #4 0x0000001754e3be72 in handle_v5 (contextp=0x17e4f8ee10, keytab=<optimized out>, fd=<optimized out>) at /usr/rtm/symbsd/src/crypto/heimdal/kadmin/server.c:551 #5 0x0000001754e3bd7a in kadmind_loop (contextp=0x17e4f8ee10, keytab=0x17e4fad330, sock=<optimized out>) at /usr/rtm/symbsd/src/crypto/heimdal/kadmin/server.c:579 #6 0x0000001754e3ccb2 in main (argc=<optimized out>, argv=<optimized out>) at /usr/rtm/symbsd/src/crypto/heimdal/kadmin/kadmind.c:202
I'll take this.
This will need more testing because I get the following in the sandbox jail used to test these kadmind problems. bob# /usr/libexec/kadmind --debug 2022-11-25T15:39:16 MODIFY: Invalid argument bob# One of the previous patches may have solved this. I'll try reverting them (in a branch of course) to see if I can reproduce this problem.
PR/267944 also fixes this PR.
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 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 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=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(-)
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(-)
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(-)