RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1937764 - free of bad pointer in XS_GSSAPI__OID_DESTROY
Summary: free of bad pointer in XS_GSSAPI__OID_DESTROY
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: perl-GSSAPI
Version: 8.3
Hardware: All
OS: Linux
medium
medium
Target Milestone: rc
: ---
Assignee: Jitka Plesnikova
QA Contact: Martin Kyral
URL:
Whiteboard:
: 1932477 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-03-11 14:17 UTC by Paulo Andrade
Modified: 2022-09-22 15:32 UTC (History)
6 users (show)

Fixed In Version: perl-GSSAPI-0.28-25.el8
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1994263 (view as bug list)
Environment:
Last Closed: 2021-11-09 18:52:05 UTC
Type: Bug
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
A different fix with copying the objects (3.07 KB, patch)
2021-08-19 14:32 UTC, Petr Pisar
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
CPAN 121873 0 None None None 2021-03-17 17:05:25 UTC
Red Hat Knowledge Base (Solution) 6977235 0 None None None 2022-09-22 15:32:48 UTC
Red Hat Product Errata RHBA-2021:4327 0 None None None 2021-11-09 18:52:06 UTC

Description Paulo Andrade 2021-03-11 14:17:50 UTC
This issue is somewhat complex, and the proper fix could be required
to be done in krb5-source-code/lib/gssapi/generic/oid_ops.c

  From gdb:

Core was generated by `/bin/perl /usr/share/doc/perl-GSSAPI/examples/gss-server.pl --port=... --keyta'.

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007f27f3d48b23 in __GI___libc_free (mem=<optimized out>)
    at malloc.c:3131
3131	  ar_ptr = arena_for_chunk (p);
(gdb) bt
#0  0x00007f27f3d48b23 in __GI___libc_free (mem=<optimized out>)
    at malloc.c:3131
#1  0x00007f27f2fe17c6 in generic_gss_release_oid (
    minor_status=minor_status@entry=0x7fffc750333c, 
    oid=oid@entry=0x7fffc7503340) at oid_ops.c:102
#2  0x00007f27f2fee6df in gss_release_oid (
    minor_status=minor_status@entry=0x7fffc750333c, 
    oid=oid@entry=0x7fffc7503340) at g_initialize.c:202
#3  0x00007f27f322f5cf in XS_GSSAPI__OID_DESTROY (my_perl=<optimized out>, 
    cv=0x564037c87130) at ./xs/OID.xs:24
#4  0x00007f27f4f58149 in Perl_pp_entersub (my_perl=0x5640378d42a0)
    at pp_hot.c:4227

  In rhel7 it just shows something like this:
*** Error in `perl': munmap_chunk(): invalid pointer: 0x00007f6cb22122be ***
======= Backtrace: =========
/lib64/libc.so.6(+0x7f3e4)[0x7f6cb940b3e4]
/lib64/libgssapi_krb5.so.2(+0xd5a2)[0x7f6cb21df5a2]
/usr/lib64/perl5/vendor_perl/auto/GSSAPI/GSSAPI.so(+0xc13c)[0x7f6cb242b13c]
/usr/lib64/perl5/CORE/libperl.so(Perl_pp_entersub+0x58f)[0x7f6cba7a642f]
/usr/lib64/perl5/CORE/libperl.so(Perl_call_sv+0x69d)[0x7f6cba73640d]
/usr/lib64/perl5/CORE/libperl.so(+0xc5185)[0x7f6cba7af185]
...

  Explaining better the issue:

OID.xs has this:
"""
void
DESTROY(oid)
	GSSAPI::OID	oid
    PREINIT:
	OM_uint32	minor;
    PPCODE:
#if !defined(HEIMDAL)
	if (oid != NULL &&
	    oid != __KRB5_MECHTYPE_OID &&
	    oid != __KRB5_OLD_MECHTYPE_OID &&
	    oid != __GSS_KRB5_NT_USER_NAME &&
	    oid != __GSS_KRB5_NT_PRINCIPAL_NAME &&
	    oid != __SPNEGO_MECHTYPE_OID &&
	    oid != __gss_mech_krb5_v2  ) {
	    (void)gss_release_oid(&minor, &oid);
	}
#endif
#if defined(HEIMDAL)
#    warn("gss_release_oid is unsupported and not Part of the API!");
#endif
"""

and in GSSAPI.xs we see:
"""
#define __KRB5_MECHTYPE_OID &mygss_mech_krb5
#define __KRB5_OLD_MECHTYPE_OID &mygss_mech_krb5_old
#define __SPNEGO_MECHTYPE_OID &myspnego_oid_desc
#define __GSS_KRB5_NT_USER_NAME &mygss_nt_krb5_name
#define __GSS_KRB5_NT_PRINCIPAL_NAME &mygss_nt_krb5_principal
#define __gss_mech_krb5_v2 &mygss_mech_krb5_v2
...
static gss_OID_desc  mygss_mech_krb5  = {9, (void *) "\x2a\x86\x48\x86\xf7\x12\x01\x02\x02"};
static gss_OID_desc  mygss_mech_krb5_old  = {5, (void *) "\x2b\x05\x01\x05\x02"};

static gss_OID_desc myspnego_oid_desc = {6, (void *) "\x2b\x06\x01\x05\x05\x02"};

static gss_OID_desc mygss_nt_krb5_name =  {10, (void *) "\052\206\110\206\367\022\001\002\002\001"};

static gss_OID_desc mygss_nt_krb5_principal = {10, (void *) "\052\206\110\206\367\022\001\002\002\002"};

static gss_OID_desc mygss_mech_krb5_v2 = {9, (void *) "\052\206\110\206\367\022\001\002\003"};
"""

so, it already checks for several possible bad free calls.

  Valgrind did show a hint of the problem:
"""
==582529== Invalid free() / delete / delete[] / realloc()
==582529==    at 0x4C320DC: free (vg_replace_malloc.c:540)
==582529==    by 0x7AA77C5: generic_gss_release_oid (oid_ops.c:102)
==582529==    by 0x78905CE: XS_GSSAPI__OID_DESTROY (OID.xs:24)
==582529==    by 0x4F27148: Perl_pp_entersub (pp_hot.c:4227)
==582529==    by 0x4E96FDD: Perl_call_sv (perl.c:2856)
==582529==    by 0x4F2C382: S_curse (sv.c:6974)
==582529==    by 0x4F2CB8F: Perl_sv_clear (sv.c:6578)
==582529==    by 0x4F2D101: Perl_sv_free2 (sv.c:7075)
==582529==    by 0x4F5EC50: Perl_leave_scope (scope.c:1178)
==582529==    by 0x4F6AEA2: Perl_pp_leave (pp_ctl.c:2121)
==582529==    by 0x4F1EF94: Perl_runops_standard (run.c:41)
==582529==    by 0x4E9EFAE: S_run_body (perl.c:2532)
==582529==    by 0x4E9EFAE: perl_run (perl.c:2455)
==582529==  Address 0x7adb6c6 is in a r-x mapped file /usr/lib64/libgssapi_krb5.so.2.2 segment
"""


  In the coredump we see:
"""
(gdb) f 1
#1  0x00007f27f2fe17c6 in generic_gss_release_oid (
    minor_status=minor_status@entry=0x7fffc750333c, 
    oid=oid@entry=0x7fffc7503340) at oid_ops.c:102
102	        free((*oid)->elements);
(gdb) p oid
$1 = (gss_OID *) 0x7fffc7503340
(gdb) p *oid
$2 = (gss_OID) 0x564037cbadb0
(gdb) p **oid
$3 = {length = 9, elements = 0x7f27f30156c6}
"""
 
and with some trial&error inspecting source code what was found is:
"""
(gdb) p gss_krb5_mechanism_oid_desc
$4 = {length = 9, elements = 0x7f27f30156c6}
(gdb) p oid.elements == gss_krb5_mechanism_oid_desc.elements
$5 = 1
"""

and the culprit should be:
"""
lib/gssapi/mechglue/g_glue.c:284

281     } else if (token->length != 0 &&
282            ((char *)token->value)[0] == 0x6E) {
283     /* Could be a raw AP-REQ (check for APPLICATION tag) */
284     *OID = gss_krb5_mechanism_oid_desc;
285     } else if (token->length == 0) {
286     *OID = gss_spnego_mechanism_oid_desc;
"""

generic_gss_release_oid() already has a check similar to the XS code,
but does not check for gss_krb5_mechanism_oid_desc. Even if did the check
it would be pointless, as it is doing a struct copy and not passing
around a reference to it (gss_krb5_mechanism_oid_desc).

  Not knowing the code and/or possible side effects, a possible fix
would be in krb5 code, like replace lib/gssapi/mechglue/g_glue.c:284
from:

    *OID = gss_krb5_mechanism_oid_desc;

with something like:

    *OID->length = gss_krb5_mechanism_oid_desc.length;
    *OID->elements = malloc(sizeof(void*) * gss_krb5_mechanism_oid_desc.length);
    memcpy(*OID->elements, gss_krb5_mechanism_oid_desc.elements, sizeof(void*) * gss_krb5_mechanism_oid_desc.length);

and do the same for other static variables where *OID is set.

  This should fix the perl-GSSAPI issue, but might cause problems elsewhere,
at least a leak on any code that might not be calling gss_release_oid
to circumvent the problem.

  Feel free to reassign the issue to krb5 component if it is more applicable.

Comment 1 Petr Pisar 2021-03-11 17:30:35 UTC
Thanks for the detailed bug report. It looks like a duplicate of bug #1932477.

Comment 3 Paulo Andrade 2021-03-11 18:01:38 UTC
Closest somewhat related information I could find around is
https://sourceforge.net/p/perlgssapi/mailman/perlgssapi-users/thread/200610051906.51257.achim%40grolmsnet.de/

Comment 4 Paulo Andrade 2021-03-11 18:09:22 UTC
BTW, my suggested pseudo patch is wrong, it is not a list of pointers,
but a non null terminated string, so, a fix would not require multiplying
the length by sizeof(void*).

Comment 5 Petr Pisar 2021-03-17 11:26:27 UTC
I confirm this issue with perl-GSSAPI-0.28-23.el8.x86_64 and krb5-libs-1.18.2-8.el8.x86_64.

Comment 6 Petr Pisar 2021-03-17 11:27:09 UTC
*** Bug 1932477 has been marked as a duplicate of this bug. ***

Comment 7 Petr Pisar 2021-03-17 17:05:27 UTC
I also came to the same conlusion that libgssapi_krb5.so copies gss_krb5_mechanism_oid_desc content into an output OID and then when asked to free it with gss_release_oid(), the library does not recognize as a static (because of a use-supplied pointer not matching any list in generic_gss_release_oid(), and attempts to free() it.

The patch is definitely wrong because gssint_get_mech_type(), where the affected OID is built, calls gssint_get_mech_type_oid() in general case. And gssint_get_mech_type_oid() also does not make deep copies. It cannot because the OID argument is not a pointer to pointer to allocate a fresh new struct gss_OID.

I found a related bug report at perl-GSSAPI upstream's bug tracker <https://rt.cpan.org/Public/Bug/Display.html?id=121873> which points to GSSAPI specification which reads that a mech_type 7th argument of agss_accept_sec_context() returns pointers to a static storage. I.e. libgssapi_krb5.so behaves correctly and it's a fault of Perl GSSAPI. The Perl module should exempt the 6th argument (and maybe others as well) of GSSAPI::Context::accept() from calls to gss_release_oid() in GSSAPI::OID destructor.

Comment 8 Petr Pisar 2021-03-17 17:12:44 UTC
A workaround for the user is pass undef to GSSAPI::Context::accept() calls:

        my $status = GSSAPI::Context::accept(
                        $server_context,
                        GSS_C_NO_CREDENTIAL,
                        $gss_input_token,
                        GSS_C_NO_CHANNEL_BINDINGS,
                        my $gss_client_name,
                    →   undef # my $out_mech,
                        my $gss_output_token,
                        my $out_flags,
                        my $out_time,
                        my $gss_delegated_cred);

Comment 9 Paulo Andrade 2021-03-18 12:33:33 UTC
  Thanks for the information.

  I guessed the patch was incorrect, and would need a full review of
other uses around, including in krb5 itself, as you have shown would
be wrong.  It was just looking at what the perl module did expect, and
I knew it likely would at least possibly create a leak on other code,
that did not call generic_gss_release_oid because it was not supposed
to, or to workaround a possible bug.

Comment 10 Petr Pisar 2021-03-18 13:48:18 UTC
Don't worry. I will come up with a patch. I will review how GSSAPI::OID object handle modifications and probably end up with making a deep copy in GSSAPI::Context::accept().

Comment 14 Michal Josef Spacek 2021-08-16 13:29:29 UTC
Summary:
Issue is in GSSAPI::OID in situation, when object is destroying and content is not destructible

 From krb5-1.19.2/src/lib/gssapi/mechglue/g_glue.c:
 static gss_OID_desc gss_krb5_mechanism_oid_desc =
         {9, (void *)"\x2a\x86\x48\x86\xf7\x12\x01\x02\x02"};

which goes to GSSAPI::OID.

In GSSAPI::OID is check for this type of content and this check isn't working.

Original code:
 From GSSAPI-0.28/xs/OID.xs:
        if (oid != NULL &&
            oid != __KRB5_MECHTYPE_OID &&
            oid != __KRB5_OLD_MECHTYPE_OID &&
            oid != __GSS_KRB5_NT_USER_NAME &&
            oid != __GSS_KRB5_NT_PRINCIPAL_NAME &&
            oid != __SPNEGO_MECHTYPE_OID &&
            oid != __gss_mech_krb5_v2  ) {
            (void)gss_release_oid(&minor, &oid);
        }

__KRB5_MECHTYPE_OID is what we need to check.

 From GSSAPI-0.28/GSSAPI.xs:
 #define __KRB5_MECHTYPE_OID &mygss_mech_krb5
 ...
 static gss_OID_desc  mygss_mech_krb5  = {9, (void *) "\x2a\x86\x48\x86\xf7\x12\x01\x02\x02"};

We need to change this check to functional version.

         if (oid != NULL &&
            ! gss_oid_equal(oid, __KRB5_MECHTYPE_OID) &&
            ! gss_oid_equal(oid, __KRB5_OLD_MECHTYPE_OID) &&
            ! gss_oid_equal(oid, __GSS_KRB5_NT_USER_NAME) &&
            ! gss_oid_equal(oid, __GSS_KRB5_NT_PRINCIPAL_NAME) &&
            ! gss_oid_equal(oid, __SPNEGO_MECHTYPE_OID) &&
            ! gss_oid_equal(oid, __gss_mech_krb5_v2) ) {
            (void)gss_release_oid(&minor, &oid);
        }

Patch in attachment.

Comment 20 Petr Pisar 2021-08-18 13:01:59 UTC
This patch is wrong. It leaks a memory if a value of an OID object accidentally matches one of the 6 exceptions. Example:

$ perl -MGSSAPI -e 'while (1) { GSSAPI::OID->from_str(GSSAPI::OID->new, gss_mech_krb5) }'

You can replace gss_mech_krb5 with "{ 1 2 840 113554 1 2 2 }" string to make this leaking reproducer more obvious.

Comment 21 Petr Pisar 2021-08-19 14:32:43 UTC
Created attachment 1815698 [details]
A different fix with copying the objects

This fix takes a different approach and does not suffer from the memory leaks.

Comment 22 Jitka Plesnikova 2021-08-20 06:55:06 UTC
I have to reopen the bug due to regression.

Comment 28 errata-xmlrpc 2021-11-09 18:52:05 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (perl-GSSAPI bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2021:4327


Note You need to log in before you can comment on or make changes to this bug.