Skip Menu |
 

History Show all quoted text — Show full headers
# Wed Apr 18 17:09:57 2018 Robbie Harwood <rharwood@redhat.com> - Ticket created
From: Robbie Harwood <rharwood@redhat.com>
To: krb5-bugs@mit.edu
Subject: Explicit NULL deref in finish_dispatch()
Date: Wed, 18 Apr 2018 16:18:25 -0400
Download (untitled) / with headers
text/plain 642B
In dispatch.c, dispatch() allocates a dispatch_state structure called
state, and initializes some fields. However, unless krb5_is_as_req(pkt)
is true, state->active_realm does not get initialized before the state
object is passed to finish_dispatch_cache.

finish_dispatch_cache() passes through state to finish_dispatch().

finish_dispatch() invokes the kdc_context macro in a call to
krb5_free_data(), which dereferences state->active_realm (for
realm_tgsprinc).

This is an explicit NULL dereference. Worth noting also is that
make_too_big_error() will attempt to dereference the same value later in
finish_dispatch().

Thanks,
--Robbie
Download signature.asc
application/pgp-signature 832B

Message body not shown because it is not plain text.

# Wed Apr 18 17:21:10 2018 ghudson@mit.edu (Greg Hudson) - Status changed from 'new' to 'open'
# Wed Apr 18 17:21:11 2018 ghudson@mit.edu (Greg Hudson) - Correspondence added
Download (untitled) / with headers
text/plain 373B
I guess this happens any time a response to a TGS request is too big to
fit into a datagram, which is rare with the DB2 or LDAP KDB module but
could easily happen with a KDB module supporting PACs.

We also need to figure out when this bug was introduced. It might have
been commit 0a2f14f752c32a24200363cc6b6ae64a92f81379 but not
necessarily. I can do that research.
# Thu Apr 19 13:31:00 2018 ghudson@mit.edu (Greg Hudson) - Subject changed from 'Explicit NULL deref in finish_dispatch()' to 'KDC null dereference when TGS reply is too big for UDP'
# Thu Apr 19 14:35:30 2018 ghudson@mit.edu (Greg Hudson) - Correspondence added
Download (untitled) / with headers
text/plain 273B
Using a test case, I verified that prior to
0a2f14f752c32a24200363cc6b6ae64a92f81379, the KDC successfully responds
to a TGS request with a too-big error if the reply length exceeds
max_dgram_reply_size, and after that commit the KDC seg faults with a
null dereference.
# Mon Apr 23 20:17:25 2018 ghudson@mit.edu (Greg Hudson) - Given to ghudson@mit.edu (Greg Hudson)
# Mon Apr 23 20:17:26 2018 ghudson@mit.edu (Greg Hudson) - Tags pullup added
# Mon Apr 23 20:17:26 2018 ghudson@mit.edu (Greg Hudson) - Status changed from 'open' to 'resolved'
# Mon Apr 23 20:17:26 2018 ghudson@mit.edu (Greg Hudson) - Target_Version 1.16-next added
# Mon Apr 23 20:17:26 2018 ghudson@mit.edu (Greg Hudson) - Target_Version 1.15-next added
# Mon Apr 23 20:17:26 2018 ghudson@mit.edu (Greg Hudson) - Correspondence added
From: ghudson@mit.edu
Subject: git commit
Download (untitled) / with headers
text/plain 1.1KiB

Fix KDC null dereference on large TGS replies

For TGS requests, dispatch() doesn't set state->active_realm, which
leads to a NULL dereference in finish_dispatch() if the reply is too
big for UDP. Prior to commit 0a2f14f752c32a24200363cc6b6ae64a92f81379
the active realm was a global and was set when process_tgs_req()
called setup_server_realm().

Move TGS decoding out of process_tgs_req() so that we can set
state->active_realm before any errors requiring response. Add a test
case.

[ghudson@mit.edu: edited commit message; added test case; reduced code
duplication; removed server handle from process_tgs_req() parameters]

https://github.com/krb5/krb5/commit/6afa8b4abf8f7c5774d03e6b15ee7288ad68d725
Author: Robbie Harwood <rharwood@redhat.com>
Committer: Greg Hudson <ghudson@mit.edu>
Commit: 6afa8b4abf8f7c5774d03e6b15ee7288ad68d725
Branch: master
src/kdc/Makefile.in | 1 +
src/kdc/dispatch.c | 48 +++++++++++++++++++++++++++---------------------
src/kdc/do_tgs_req.c | 24 ++++++------------------
src/kdc/kdc_util.h | 5 ++---
src/kdc/t_bigreply.py | 19 +++++++++++++++++++
5 files changed, 55 insertions(+), 42 deletions(-)
# Wed May 02 01:25:43 2018 ghudson@mit.edu (Greg Hudson) - Version_Fixed 1.16.1 added
# Wed May 02 01:25:43 2018 ghudson@mit.edu (Greg Hudson) - Correspondence added
From: ghudson@mit.edu
Subject: git commit
Download (untitled) / with headers
text/plain 1.2KiB

Fix KDC null dereference on large TGS replies

For TGS requests, dispatch() doesn't set state->active_realm, which
leads to a NULL dereference in finish_dispatch() if the reply is too
big for UDP. Prior to commit 0a2f14f752c32a24200363cc6b6ae64a92f81379
the active realm was a global and was set when process_tgs_req()
called setup_server_realm().

Move TGS decoding out of process_tgs_req() so that we can set
state->active_realm before any errors requiring response. Add a test
case.

[ghudson@mit.edu: edited commit message; added test case; reduced code
duplication; removed server handle from process_tgs_req() parameters]

(cherry picked from commit 6afa8b4abf8f7c5774d03e6b15ee7288ad68d725)

https://github.com/krb5/krb5/commit/086b505b7248ec78502857d6dac72a57c59b36e8
Author: Robbie Harwood <rharwood@redhat.com>
Committer: Greg Hudson <ghudson@mit.edu>
Commit: 086b505b7248ec78502857d6dac72a57c59b36e8
Branch: krb5-1.16
src/kdc/Makefile.in | 1 +
src/kdc/dispatch.c | 48 +++++++++++++++++++++++++++---------------------
src/kdc/do_tgs_req.c | 24 ++++++------------------
src/kdc/kdc_util.h | 5 ++---
src/kdc/t_bigreply.py | 19 +++++++++++++++++++
5 files changed, 55 insertions(+), 42 deletions(-)
# Wed May 02 01:26:06 2018 ghudson@mit.edu (Greg Hudson) - Version_Fixed 1.15.3 added
# Wed May 02 01:26:06 2018 ghudson@mit.edu (Greg Hudson) - Correspondence added
From: ghudson@mit.edu
Subject: git commit
Download (untitled) / with headers
text/plain 1.1KiB

Fix KDC null dereference on large TGS replies

For TGS requests, dispatch() doesn't set state->active_realm, which
leads to a NULL dereference in finish_dispatch() if the reply is too
big for UDP. Prior to commit 0a2f14f752c32a24200363cc6b6ae64a92f81379
the active realm was a global and was set when process_tgs_req()
called setup_server_realm().

Move TGS decoding out of process_tgs_req() so that we can set
state->active_realm before any errors requiring response. Add a test
case.

[ghudson@mit.edu: edited commit message; added test case; reduced code
duplication; removed server handle from process_tgs_req() parameters]

(cherry picked from commit 6afa8b4abf8f7c5774d03e6b15ee7288ad68d725)

https://github.com/krb5/krb5/commit/be1e46f32c603cde7880cf07ed830a7320e959e1
Author: Robbie Harwood <rharwood@redhat.com>
Committer: Greg Hudson <ghudson@mit.edu>
Commit: be1e46f32c603cde7880cf07ed830a7320e959e1
Branch: krb5-1.15
src/kdc/Makefile.in | 1 +
src/kdc/dispatch.c | 46 ++++++++++++++++++++++++++--------------------
src/kdc/do_tgs_req.c | 24 ++++++------------------
src/kdc/kdc_util.h | 5 ++---
src/kdc/t_bigreply.py | 12 ++++++++++++
5 files changed, 47 insertions(+), 41 deletions(-)
# Wed May 02 10:16:07 2018 ghudson@mit.edu (Greg Hudson) - CustomField pullup deleted