Skip Menu |
 

From: "Krier, Richard" <Richard.Krier@globalfoundries.com>
To: "krb5-bugs@mit.edu" <krb5-bugs@mit.edu>
Date: Wed, 8 Sep 2010 09:26:46 -0500
Subject: Segmentation fault in krb library (sn2princ.c) if realm not resolved
CC: "Adhikari, Diwas" <diwas.adhikari@globalfoundries.com>, "Pinto, Kevin" <kevin.pinto@globalfoundries.com>
Download (untitled) / with headers
text/plain 2.2KiB
Download (untitled) / with headers
text/html 11.3KiB

  To: krb5-bugs@mit.edu

  Subject: Segmentation fault in krb library (sn2princ.c) if realm not resolved

  From: Richard.Krier@globalfoundries.com

  Reply-To: Richard.Krier@globalfoundries.com

  Cc:

  X-send-pr-version: 3.99

       

     >Submitter-Id:   

     >Originator:      Richard Krier

     >Organization:    GlobalFoundries

    

     >Confidential:    no

     >Synopsis: Segmentation fault in sn2princ.c if realm not resolved. Need checks for zero-length string and/or NULL pointer

     >Severity: serious

     >Priority: medium

     >Category: krb5-bug

     >Class:           krb5-bug

     >Release: 1.6.3, 1.8.3

     >Environment:

        <machine, os, target, libraries (multiple lines)>

     System: AIX 5.3, Kerberos libraries built in 64-bit mode

     Machine:

     >Description:

              1. sname_to_princ() (sn2princ.c) calls krb5_get_host_realm() to resolve kerberos realm from host name.

              2. If realm unresolved, krb5_get_host_realm() returns a zero-length string, i.e. 1 byte containing just ‘\0’

              3. sname_to_princ() then performs inadequate check for realm resolution:

PROBLEM IS HERE:   if (!hrealms[0]) {   /* this only checks if ptr is NULL, but not if string is zero-length */

                      free(remote_host);

                      krb5_xfree(hrealms);

                      return KRB5_ERR_HOST_REALM_UNKNOWN;

                    }

              3. sname_to_princ() then calls krb5_build_principal(), principal not created in this case, *ret_princ is NULL

ALSO HERE:       No check is made to determine if *ret_princ is NULL before using it to make an assignment as point 4.

              4. sname_to_princ() gets segmentation fault trying to use null *ret_princ to assign ‘type’

 

 

     >How-To-Repeat:

             N/A

     >Fix:

             In file sn2princ.c:

             1. Modify the realm-check above to check for either a NULL pointer or a zero-length string:

                if ( (!hrealms[0]) || (0==strlen(hrealms[0]) )    /* or perhaps   (‘\0’==hrealms[0]) */

 

            2. Add a check for *ret_princ being NULL after calling krb5_build_principal()

                    if (NULL==*ret_princ{

                      free(remote_host);

                      krb5_xfree(hrealms);

                      return KRB5_ERR_HOST_REALM_UNKNOWN;

                    }

 

 

[Richard.Krier@globalfoundries.com - Tue Sep 14 15:10:55 2010]:
Show quoted text
> 2. If realm unresolved, krb5_get_host_realm() returns a
> zero-length string, i.e. 1 byte containing just '\0'

This is not actually an error condition. The empty string is returned
here to indicate that the caller should try referrals. So step 3 is
actually correct.

Show quoted text
> 3. sname_to_princ() then calls krb5_build_principal(),
> principal not created in this case, *ret_princ is NULL
> ALSO HERE: No check is made to determine if *ret_princ is NULL
> before using it to make an assignment as point 4.

Failing to check retval here is a bug, and we'll fix it. However,
krb5_build_principal() should succeed in the scenario you described; I
would be interested to know why it is failing in your use case.
From: ghudson@mit.edu
Subject: SVN Commit

In krb5_sname_to_principal, correctly handle failures from
krb5_build_principal.


https://github.com/krb5/krb5/commit/073c9305ed0eff74c06ed8fd1b950fd47828d1e5
Commit By: ghudson
Revision: 24309
Changed Files:
U trunk/src/lib/krb5/os/sn2princ.c
From: "Krier, Richard" <Richard.Krier@globalfoundries.com>
To: "rt-comment@krbdev.mit.edu" <rt-comment@krbdev.mit.edu>
CC: "Adhikari, Diwas" <Diwas.Adhikari@globalfoundries.com>, "Pinto, Kevin" <Kevin.Pinto@globalfoundries.com>
Date: Wed, 15 Sep 2010 13:20:26 -0500
Subject: RE: [krbdev.mit.edu #6777] Segmentation fault in krb library (sn2princ.c) if realm not resolved
RT-Send-Cc:
Download (untitled) / with headers
text/plain 3.5KiB
Hello Greg,

Thanks for the feedback.

Recently we built the krb5-1.6.3 kerberos libraries on AIX 5.3 in 64-bit mode for use with a manufacturing application. Previously we built them in 32-bit mode for an earlier version of the application. We use both versions at this time, as the 64-bit version is just being prepared for use in production.

1. Adding a check in sname_to_princ() for null *ret_princ coming back from krb5_build_principal would prevent the
segmentation fault we experienced. Thanks for accepting that recommendation. Can you tell me when and how we could obtain
the fixed version of this source file?

2. As for krb5_build_principal(), seems the code is set up to return null ret_princ if the input realm is missing,
think it is working as designed.

3. I think the root of our problem is related to building the code in 64-bit mode; krb5_get_host_realm() behaves
differently in 32-bit and 64-bit mode in the case where the realm cannot be resolved from the host name:
a. In 32-bit mode, it properly returns the default_realm value specified value specified in [libdefaults] stanza.
b. In 64-bit mode, it only returns the zero-length string

4. Possibly the parsing code for krb5.conf is not correct in 64bit mode, perhaps due to the lengths of pointers. longs,
etc - it may be incurring some typical issues when migrating legacy C/C++ code from 32-bit to 64-bit.
a. I did not have a chance to investigate the parsing code. Thus far we've avoided making changes to the kerberos
source, trying only to build the libraries using the as-downloaded code, using the AIX xlC_r compiler in 64-bit mode.
b. To build in 64-bit mode, I set the following environment variables before running make:
export OBJECT_MODE=64
export LDFLAGS='-brtl -L$(DB2DIR)/lib64'

5. We created a small stand-alone test program (krb5test) to simulate how our application interacts with kerberos and to
reproduce the problem, and to compare 32-bit and 64-bit results. I can ask for permission to send it to you if you would
find it useful.

6. Also have a version of 3 or 4 kerberos source files in which we added some debug tracing to track down the crash
problem; we only used this modified version in our development environment to identify why the segmentation fault was
occurring. Otherwise we've only used unmodified source. For further investigation of the parsing code issue, would extend
this tracing a little farther into that source file.

Regards,
Richard



Show quoted text
-----Original Message-----
From: Greg Hudson via RT [mailto:rt-comment@krbdev.mit.edu]
Sent: Tuesday, September 14, 2010 6:07 PM
To: Krier, Richard
Subject: [krbdev.mit.edu #6777] Segmentation fault in krb library (sn2princ.c) if realm not resolved

[Richard.Krier@globalfoundries.com - Tue Sep 14 15:10:55 2010]:
> 2. If realm unresolved, krb5_get_host_realm() returns a
> zero-length string, i.e. 1 byte containing just '\0'

This is not actually an error condition. The empty string is returned
here to indicate that the caller should try referrals. So step 3 is
actually correct.

> 3. sname_to_princ() then calls krb5_build_principal(),
> principal not created in this case, *ret_princ is NULL
> ALSO HERE: No check is made to determine if *ret_princ is NULL
> before using it to make an assignment as point 4.

Failing to check retval here is a bug, and we'll fix it. However,
krb5_build_principal() should succeed in the scenario you described; I
would be interested to know why it is failing in your use case.
From: "Krier, Richard" <Richard.Krier@globalfoundries.com>
To: "rt-comment@krbdev.mit.edu" <rt-comment@krbdev.mit.edu>
Date: Wed, 15 Sep 2010 13:37:54 -0500
Subject: FW: [krbdev.mit.edu #6777] Segmentation fault in krb library (sn2princ.c) if realm not resolved
RT-Send-Cc:
Greg,

Thanks for making the fix so quickly; your mail arrived while I was writing to you.

I think it would be worth trying to fix the issue where krb5_get_host_realm() is not returning the default_realm in 64bit mode.
My guess is it relates to the parsing code, or somewhere close by.

Richard

Show quoted text
-----Original Message-----
From: Krier, Richard
Sent: Wednesday, September 15, 2010 2:20 PM
To: 'rt-comment@krbdev.mit.edu'
Cc: Adhikari, Diwas; Pinto, Kevin
Subject: RE: [krbdev.mit.edu #6777] Segmentation fault in krb library (sn2princ.c) if realm not resolved

Hello Greg,

Thanks for the feedback.

Recently we built the krb5-1.6.3 kerberos libraries on AIX 5.3 in 64-bit mode for use with a manufacturing application. Previously we built them in 32-bit mode for an earlier version of the application. We use both versions at this time, as the 64-bit version is just being prepared for use in production.

1. Adding a check in sname_to_princ() for null *ret_princ coming back from krb5_build_principal would prevent the
segmentation fault we experienced. Thanks for accepting that recommendation. Can you tell me when and how we could obtain
the fixed version of this source file?

2. As for krb5_build_principal(), seems the code is set up to return null ret_princ if the input realm is missing,
think it is working as designed.

3. I think the root of our problem is related to building the code in 64-bit mode; krb5_get_host_realm() behaves
differently in 32-bit and 64-bit mode in the case where the realm cannot be resolved from the host name:
a. In 32-bit mode, it properly returns the default_realm value specified value specified in [libdefaults] stanza.
b. In 64-bit mode, it only returns the zero-length string

4. Possibly the parsing code for krb5.conf is not correct in 64bit mode, perhaps due to the lengths of pointers. longs,
etc - it may be incurring some typical issues when migrating legacy C/C++ code from 32-bit to 64-bit.
a. I did not have a chance to investigate the parsing code. Thus far we've avoided making changes to the kerberos
source, trying only to build the libraries using the as-downloaded code, using the AIX xlC_r compiler in 64-bit mode.
b. To build in 64-bit mode, I set the following environment variables before running make:
export OBJECT_MODE=64
export LDFLAGS='-brtl -L$(DB2DIR)/lib64'

5. We created a small stand-alone test program (krb5test) to simulate how our application interacts with kerberos and to
reproduce the problem, and to compare 32-bit and 64-bit results. I can ask for permission to send it to you if you would
find it useful.

6. Also have a version of 3 or 4 kerberos source files in which we added some debug tracing to track down the crash
problem; we only used this modified version in our development environment to identify why the segmentation fault was
occurring. Otherwise we've only used unmodified source. For further investigation of the parsing code issue, would extend
this tracing a little farther into that source file.

Regards,
Richard



-----Original Message-----
From: Greg Hudson via RT [mailto:rt-comment@krbdev.mit.edu]
Sent: Tuesday, September 14, 2010 6:07 PM
To: Krier, Richard
Subject: [krbdev.mit.edu #6777] Segmentation fault in krb library (sn2princ.c) if realm not resolved

[Richard.Krier@globalfoundries.com - Tue Sep 14 15:10:55 2010]:
> 2. If realm unresolved, krb5_get_host_realm() returns a
> zero-length string, i.e. 1 byte containing just '\0'

This is not actually an error condition. The empty string is returned
here to indicate that the caller should try referrals. So step 3 is
actually correct.

> 3. sname_to_princ() then calls krb5_build_principal(),
> principal not created in this case, *ret_princ is NULL
> ALSO HERE: No check is made to determine if *ret_princ is NULL
> before using it to make an assignment as point 4.

Failing to check retval here is a bug, and we'll fix it. However,
krb5_build_principal() should succeed in the scenario you described; I
would be interested to know why it is failing in your use case.
Download (untitled) / with headers
text/plain 1.5KiB
[Richard.Krier@globalfoundries.com - Wed Sep 15 14:20:35 2010]:
Show quoted text
> 2. As for krb5_build_principal(), seems the code is set up to return
> null ret_princ if the input realm is missing,
> think it is working as designed.

There is no code like that in our tree. krb5_build_principal() should
be able to build a principal with an empty realm. If
krb5_build_principal() is failing with an empty realm, we will not
understand your issue until you investigate why.

Show quoted text
> 3. I think the root of our problem is related to building the code in
> 64-bit mode; krb5_get_host_realm() behaves
> differently in 32-bit and 64-bit mode in the case where the realm
> cannot be resolved from the host name:
> a. In 32-bit mode, it properly returns the default_realm value
> specified value specified in [libdefaults] stanza.
> b. In 64-bit mode, it only returns the zero-length string

The behavior of krb5_get_host_realm() changed in krb5 1.6. The new
design is that it will return an empty realm if there is no explicit
krb5.conf configuration mapping the domain to a realm. This is a cue to
krb5_get_credentials that it should try KDC referrals against the local
realm. If that doesn't work, krb5_get_credentials will invoke
krb5_get_fallback_host_realm() to perform DNS-based or heuristic methods
to determine the realm name, eventually falling back to the default
realm.

If you are seeing different behavior from krb5_get_host_realm() on 32-
bit and 64-bit, then it is probably because you are getting different
versions of the krb5 libraries for some reason.
To: rt@krbdev.MIT.EDU
Subject: Re: [krbdev.mit.edu #6777] Segmentation fault in krb library (sn2princ.c) if realm not resolved
From: Tom Yu <tlyu@MIT.EDU>
Date: Fri, 17 Sep 2010 14:46:42 -0400
RT-Send-Cc:
"Greg Hudson via RT" <rt-comment@krbdev.mit.edu> writes:

Show quoted text
> [Richard.Krier@globalfoundries.com - Wed Sep 15 14:20:35 2010]:
>> 2. As for krb5_build_principal(), seems the code is set up to return
>> null ret_princ if the input realm is missing,
>> think it is working as designed.
>
> There is no code like that in our tree. krb5_build_principal() should
> be able to build a principal with an empty realm. If
> krb5_build_principal() is failing with an empty realm, we will not
> understand your issue until you investigate why.

Perhaps strdup() on AIX can return NULL for a zero-length string? For
that matter, does malloc() on AIX return NULL for a zero argument?
[tlyu - Fri Sep 17 14:46:43 2010]:
Show quoted text
> Perhaps strdup() on AIX can return NULL for a zero-length string? For
> that matter, does malloc() on AIX return NULL for a zero argument?

I don't think it's fruitful to speculate, only to step through in a
debugger.

(krb5int_build_principal_va() would not invoke malloc(0) if the realm is
empty. It would invoke strdup("") but a lot more software would break on
AIX if it's strdup were so broken as to return NULL in that case.)
To: rt@krbdev.MIT.EDU
Subject: Re: [krbdev.mit.edu #6777] Segmentation fault in krb library (sn2princ.c) if realm not resolved
From: Tom Yu <tlyu@MIT.EDU>
Date: Fri, 17 Sep 2010 15:30:45 -0400
RT-Send-Cc:
"Greg Hudson via RT" <rt-comment@krbdev.mit.edu> writes:

Show quoted text
> [tlyu - Fri Sep 17 14:46:43 2010]:
>> Perhaps strdup() on AIX can return NULL for a zero-length string? For
>> that matter, does malloc() on AIX return NULL for a zero argument?
>
> I don't think it's fruitful to speculate, only to step through in a
> debugger.
>
> (krb5int_build_principal_va() would not invoke malloc(0) if the realm is
> empty. It would invoke strdup("") but a lot more software would break on
> AIX if it's strdup were so broken as to return NULL in that case.)

Actually krb5_build_principal_va() in krb5-1.6.x (the krb5int_ version
is newer than 1.6) does execute malloc(rlen) and check it for NULL.
Oh, I missed that. In that case, speculation is much more fruitful.
It's very likely a malloc(0) --> NULL issue.

The easiest fix for Richard would be to change line 51 of bld_princ.c to
say "tmpdata = malloc(rlen + 1)" instead of "tmpdata = malloc(rlen)".