Skip Menu |
 

Subject: gss_add_cred() aliases memory when creating extended cred, leading to memory bugs
Download (untitled) / with headers
text/plain 1.2KiB
gss_add_cred() has an input_cred_handle parameter and and
output_cred_handle parameter, both of which are optional (but the
caller has to specify at least one). Per RFC 2744, it can be used in
three modes:

1. Create a new cred handle with one mech cred (output with no input)
2. Add a mech cred to an existing cred handle (input with no output)
3. "compose a new credential containing all credential-elements of
the original in addition to the newly-acquire credential-element"
(input and output)

The first two cases work (except for a memory leak in case 1 as
reported in ticket 8729), but in the third case we make a shallow
copy of the existing OIDs and mechanism credentials from the input
handle. Once one of the cred handles is released, the other cred
handle contains invalid pointers which are then incorrectly accessed
when the other handle is used or released (use after free and/or
double free).

This bug is quite old, so it seems unlikely that gss_add_cred() is
being used this way in programs, but we should of course fix it. The
fix will be kind of awkward, as GSSAPI provides no way to copy cred
handles. We now have gss_export_cred() and gss_import_cred(); we can
use those to inefficiently copy credentials under the assumption that
this will not be a common operation.
From: ghudson@mit.edu
Subject: git commit
Download (untitled) / with headers
text/plain 1.4KiB

Fix memory bugs in gss_add_cred() extension case

If gss_add_cred() is called with both an input_cred_handle and an
output_cred_handle, it creates a new credential with the elements of
the input credential plus the requested element. Making a shallow
copy of mechs_array and cred_array from the old credential creates
aliased pointers which become invalid when one of the two credentials
is released, leading to use-after-free and double-free errors.

Instead, make a full copy of the input cred for this case. Make this
copy at the beginning so that union_cred can always be modified in
place (and freed on error using gss_release_cred() if we created it),
removing the need for new_union_cred, new_mechs_array, and
new_cred_array. Use a stack object for target_mechs to simplify
cleanup and reduce the number of failure cases.

GSSAPI provides no facility for copying a credential; since we mostly
use the GSSAPI as our SPI for mechanisms, we have no simple way to
copy mechanism creds when copying the union cred. Use
gss_export_cred() and gss_import_cred() if the mechanism provides
them; otherwise fall back to gss_inquire_cred() and
gss_acquire_cred().

https://github.com/krb5/krb5/commit/288cbada833dc6af7d43dd308563b48b73347dfb
Author: Greg Hudson <ghudson@mit.edu>
Commit: 288cbada833dc6af7d43dd308563b48b73347dfb
Branch: master
src/lib/gssapi/mechglue/g_acquire_cred.c | 207 ++++++++++++++++++++----------
src/tests/gssapi/t_add_cred.c | 31 +++++-
2 files changed, 167 insertions(+), 71 deletions(-)
From: ghudson@mit.edu
Subject: git commit
Download (untitled) / with headers
text/plain 1.5KiB

Fix memory bugs in gss_add_cred() extension case

If gss_add_cred() is called with both an input_cred_handle and an
output_cred_handle, it creates a new credential with the elements of
the input credential plus the requested element. Making a shallow
copy of mechs_array and cred_array from the old credential creates
aliased pointers which become invalid when one of the two credentials
is released, leading to use-after-free and double-free errors.

Instead, make a full copy of the input cred for this case. Make this
copy at the beginning so that union_cred can always be modified in
place (and freed on error using gss_release_cred() if we created it),
removing the need for new_union_cred, new_mechs_array, and
new_cred_array. Use a stack object for target_mechs to simplify
cleanup and reduce the number of failure cases.

GSSAPI provides no facility for copying a credential; since we mostly
use the GSSAPI as our SPI for mechanisms, we have no simple way to
copy mechanism creds when copying the union cred. Use
gss_export_cred() and gss_import_cred() if the mechanism provides
them; otherwise fall back to gss_inquire_cred() and
gss_acquire_cred().

(cherry picked from commit 288cbada833dc6af7d43dd308563b48b73347dfb)

https://github.com/krb5/krb5/commit/71d28787a8f70186909141c8b6de2e7798ab5e41
Author: Greg Hudson <ghudson@mit.edu>
Commit: 71d28787a8f70186909141c8b6de2e7798ab5e41
Branch: krb5-1.16
src/lib/gssapi/mechglue/g_acquire_cred.c | 207 ++++++++++++++++++++----------
src/tests/gssapi/t_add_cred.c | 31 +++++-
2 files changed, 167 insertions(+), 71 deletions(-)
From: ghudson@mit.edu
Subject: git commit
Download (untitled) / with headers
text/plain 1.5KiB

Fix memory bugs in gss_add_cred() extension case

If gss_add_cred() is called with both an input_cred_handle and an
output_cred_handle, it creates a new credential with the elements of
the input credential plus the requested element. Making a shallow
copy of mechs_array and cred_array from the old credential creates
aliased pointers which become invalid when one of the two credentials
is released, leading to use-after-free and double-free errors.

Instead, make a full copy of the input cred for this case. Make this
copy at the beginning so that union_cred can always be modified in
place (and freed on error using gss_release_cred() if we created it),
removing the need for new_union_cred, new_mechs_array, and
new_cred_array. Use a stack object for target_mechs to simplify
cleanup and reduce the number of failure cases.

GSSAPI provides no facility for copying a credential; since we mostly
use the GSSAPI as our SPI for mechanisms, we have no simple way to
copy mechanism creds when copying the union cred. Use
gss_export_cred() and gss_import_cred() if the mechanism provides
them; otherwise fall back to gss_inquire_cred() and
gss_acquire_cred().

(cherry picked from commit 288cbada833dc6af7d43dd308563b48b73347dfb)

https://github.com/krb5/krb5/commit/697458053ed317364ee507c9497e148e9d1aa7ab
Author: Greg Hudson <ghudson@mit.edu>
Commit: 697458053ed317364ee507c9497e148e9d1aa7ab
Branch: krb5-1.15
src/lib/gssapi/mechglue/g_acquire_cred.c | 207 ++++++++++++++++++++----------
src/tests/gssapi/t_add_cred.c | 31 +++++-
2 files changed, 167 insertions(+), 71 deletions(-)