[krbdev.mit.edu #8567] Bug in mslsa ccahe

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[krbdev.mit.edu #8567] Bug in mslsa ccahe

Greg Hudson via RT
   Hi

   I believe I've found a bug in mit krb. The bug is in krb5_lcc_data()
in src/lib/krb5/ccache/cc_mslsa.c.

When krb5_lcc_data is allocated data->flags is not initialized. As
result krb5_lcc_next_cred() may not copy the ticket if flags might
happened to have KRB5_TC_NOTICKET bit randomly set.

Here is a simple fix:

diff --git a/src/lib/krb5/ccache/cc_mslsa.c b/src/lib/krb5/ccache/cc_mslsa.c
index 7a80470..c741a50 100644
--- a/src/lib/krb5/ccache/cc_mslsa.c
+++ b/src/lib/krb5/ccache/cc_mslsa.c
@@ -1553,6 +1553,7 @@ krb5_lcc_resolve (krb5_context context,
krb5_ccache *id, const char *residual)
      data->LogonHandle = LogonHandle;
      data->PackageId = PackageId;
      data->princ = NULL;
+    data->flags = 0;

      data->cc_name = (char *)malloc(strlen(residual)+1);
      if (data->cc_name == NULL) {


   Regards

   Alex.

--
Alexander D. Karaivanov, System Developer | Karos Health, Krumtappen 4,3.th,2500 Valby, Denmark
Phone:+ 45 46550444, Mobile: +45 40995501 | skype: alexander.karaivanov, gtalk: [hidden email]


_______________________________________________
krb5-bugs mailing list
[hidden email]
https://mailman.mit.edu/mailman/listinfo/krb5-bugs
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [krbdev.mit.edu #8567] Bug in mslsa ccahe

Greg Hudson via RT
On Fri, Mar 31, 2017 at 12:06:53AM -0400, Alexander Karaivanov via RT wrote:

>    Hi
>
>    I believe I've found a bug in mit krb. The bug is in krb5_lcc_data()
> in src/lib/krb5/ccache/cc_mslsa.c.
>
> When krb5_lcc_data is allocated data->flags is not initialized. As
> result krb5_lcc_next_cred() may not copy the ticket if flags might
> happened to have KRB5_TC_NOTICKET bit randomly set.
>
> Here is a simple fix:
>
> diff --git a/src/lib/krb5/ccache/cc_mslsa.c b/src/lib/krb5/ccache/cc_mslsa.c
> index 7a80470..c741a50 100644
> --- a/src/lib/krb5/ccache/cc_mslsa.c
> +++ b/src/lib/krb5/ccache/cc_mslsa.c
> @@ -1553,6 +1553,7 @@ krb5_lcc_resolve (krb5_context context,
> krb5_ccache *id, const char *residual)
>       data->LogonHandle = LogonHandle;
>       data->PackageId = PackageId;
>       data->princ = NULL;
> +    data->flags = 0;
>
>       data->cc_name = (char *)malloc(strlen(residual)+1);
>       if (data->cc_name == NULL) {

One could argue whether we should just zero the entire allocation
(and drop the princ and flags initialization as redundant), but on
first look this seems to generally be the right thing to do.

-Ben

_______________________________________________
krb5-bugs mailing list
[hidden email]
https://mailman.mit.edu/mailman/listinfo/krb5-bugs
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [krbdev.mit.edu #8567] Bug in mslsa ccahe

Greg Hudson via RT
In reply to this post by Greg Hudson via RT
On 04/06/2017 04:47 AM, Benjamin Kaduk via RT wrote:

> On Fri, Mar 31, 2017 at 12:06:53AM -0400, Alexander Karaivanov via RT wrote:
>>     Hi
>>
>>     I believe I've found a bug in mit krb. The bug is in krb5_lcc_data()
>> in src/lib/krb5/ccache/cc_mslsa.c.
>>
>> When krb5_lcc_data is allocated data->flags is not initialized. As
>> result krb5_lcc_next_cred() may not copy the ticket if flags might
>> happened to have KRB5_TC_NOTICKET bit randomly set.
>>
>> Here is a simple fix:
>>
>> diff --git a/src/lib/krb5/ccache/cc_mslsa.c b/src/lib/krb5/ccache/cc_mslsa.c
>> index 7a80470..c741a50 100644
>> --- a/src/lib/krb5/ccache/cc_mslsa.c
>> +++ b/src/lib/krb5/ccache/cc_mslsa.c
>> @@ -1553,6 +1553,7 @@ krb5_lcc_resolve (krb5_context context,
>> krb5_ccache *id, const char *residual)
>>        data->LogonHandle = LogonHandle;
>>        data->PackageId = PackageId;
>>        data->princ = NULL;
>> +    data->flags = 0;
>>
>>        data->cc_name = (char *)malloc(strlen(residual)+1);
>>        if (data->cc_name == NULL) {
> One could argue whether we should just zero the entire allocation
> (and drop the princ and flags initialization as redundant), but on
> first look this seems to generally be the right thing to do.
>
> -Ben
That was my first thought too, as to be on the safe side, but then I thought, one better decide for every (potentially added in future new) variable of the
structure what is the correct default/initial value... Zero may or may not be the correct one.

Alex.


--
Alexander D. Karaivanov, System Developer
Karos Health, Krumtappen 4,3.th,2500 Valby, Denmark | Phone:+ 45 46550444, Mobile: +45 40995501
skype: alexander.karaivanov, gtalk: [hidden email]


_______________________________________________
krb5-bugs mailing list
[hidden email]
https://mailman.mit.edu/mailman/listinfo/krb5-bugs
Loading...