RE: Heap Corruption with large authorization header values

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RE: Heap Corruption with large authorization header values

Tim Vega
First a clarification, this is krb5-1.12.1, not 1.12.2

We've made some progress and found that what's happening is a memory allocation/free function mismatch:

Line 1241 of src\lib\gssapi\krb5\accept_sec_context.c:
token.value = (unsigned char *) xmalloc(token.length);

This allocates the token which is then deallocated here:

Line 1790 of src\lib\gssapi\spnego\spnego_mech.c
gss_release_buffer(&tmpmin, &mechtok_out);

Changing xmalloc to gssalloc_malloc solves our issue.

May I forward this to the bug-reports mailing list?

Tim

From: Tim Vega
Sent: Wednesday, October 01, 2014 1:19 PM
To: [hidden email]
Subject: Heap Corruption with large authorization header values

Hello,

We have mod_auth_kerb 5.4 running with krb5-1.12.2

When sending a request with a very large authorization value, 12462 characters and sample attached, the kerberos library encounters a heap corruption somewhere in a call to gss_accept_sec_context.
The data that appears to be corrupted is pointed to by the variable mechtok_out in spnego_gss_accept_sec_context in lib/gssapi/spnego/spnego_mech.c. The corruption gets detected by a call to gss_release_buffer in the cleanup routine of the same function.

Has anyone seen this before? Is this expected behavior given a large auth header?

Tim


_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: Heap Corruption with large authorization header values

Greg Hudson
[I've moderated Tim's messages through to krbdev.  I don't plan to
moderate them through to kerberos; messages should go to one or the
other, not both.]

On 10/02/2014 04:25 PM, Tim Vega wrote:
> Line 1241 of src\lib\gssapi\krb5\accept_sec_context.c:
> token.value = (unsigned char *) xmalloc(token.length);
>
> This allocates the token which is then deallocated here:
>
> Line 1790 of src\lib\gssapi\spnego\spnego_mech.c
> gss_release_buffer(&tmpmin, &mechtok_out);
>
> Changing xmalloc to gssalloc_malloc solves our issue.

I assume you're using a build from source on Windows?

I agree with the description of the bug; this malloc call should have
been converted when we introduced gssalloc_malloc.  The bug can't
manifest in 1.10.x (and thus in the most recent Kerberos for Windows
release) because it's masked by #1445, which was fixed in 1.12:
http://krbdev.mit.edu/rt/Ticket/Display.html?id=1445

I will go ahead and submit a fix for this; no need to send a separate
bug report.

_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

RE: Heap Corruption with large authorization header values

Tim Vega
Hi Greg,

We ran into another bug of a similar nature

Line 868 of src\lib\gssapi\krb5\accept_sec_context.c:
if ((ctx = (krb5_gss_ctx_id_rec *) xmalloc(sizeof(krb5_gss_ctx_id_rec)))

Would it also be safe to change this xmalloc call to gssalloc_malloc? We're concerned with running into more of the same as well. Is there some sort of systematic rule we can use to determine if it's safe to convert xmalloc calls to gssalloc_malloc?

Tim

-----Original Message-----
From: Greg Hudson [mailto:[hidden email]]
Sent: Tuesday, October 07, 2014 5:23 PM
To: Tim Vega; [hidden email]
Subject: Re: Heap Corruption with large authorization header values

[I've moderated Tim's messages through to krbdev.  I don't plan to moderate them through to kerberos; messages should go to one or the other, not both.]

On 10/02/2014 04:25 PM, Tim Vega wrote:
> Line 1241 of src\lib\gssapi\krb5\accept_sec_context.c:
> token.value = (unsigned char *) xmalloc(token.length);
>
> This allocates the token which is then deallocated here:
>
> Line 1790 of src\lib\gssapi\spnego\spnego_mech.c
> gss_release_buffer(&tmpmin, &mechtok_out);
>
> Changing xmalloc to gssalloc_malloc solves our issue.

I assume you're using a build from source on Windows?

I agree with the description of the bug; this malloc call should have been converted when we introduced gssalloc_malloc.  The bug can't manifest in 1.10.x (and thus in the most recent Kerberos for Windows
release) because it's masked by #1445, which was fixed in 1.12:
http://krbdev.mit.edu/rt/Ticket/Display.html?id=1445

I will go ahead and submit a fix for this; no need to send a separate bug report.


_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: Heap Corruption with large authorization header values

Greg Hudson
On 11/12/2014 06:58 PM, Tim Vega wrote:
> We ran into another bug of a similar nature
>
> Line 868 of src\lib\gssapi\krb5\accept_sec_context.c:
> if ((ctx = (krb5_gss_ctx_id_rec *) xmalloc(sizeof(krb5_gss_ctx_id_rec)))

I don't think that's a bug.  krb5_gss_ctx_id_rec structures are freed
within the krb5 mech with xfree (at delete_sec_context.c line 91), not
in the mechglue with gssalloc_free.

> Would it also be safe to change this xmalloc call to gssalloc_malloc? We're concerned with running into more of the same as well. Is there some sort of systematic rule we can use to determine if it's safe to convert xmalloc calls to gssalloc_malloc?

A gss_buffer_desc value returned to the caller must use gssalloc_malloc.
 Everything else should continue using malloc/free.  (xmalloc/xfree are
just #defines for malloc and free, which we will probably get rid of at
some point.)
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev