Short read from krb5_rc_io_read

[JC Ferguson]

line 356 also references "maxlen".
Where is that initialized?

If you set "len2" to zero when you declare
it, does the warning go away?

 

> -----Original Message-----
> From: krbdev-bounces at mit.edu [mailto:krbdev-bounces at mit.edu] 
> On Behalf Of Mike Patnode
> Sent: Friday, March 14, 2008 12:42
> To: Kerberos developers list
> Subject: Short read from krb5_rc_io_read
> 
> 
> I'm currently running valgrind on code build with 1.4.3, but 
> I don't see
> any difference here in the 1.6 source base.    I'm getting 
> the following
> error:
> 
> 
> ==7938== Conditional jump or move depends on uninitialised value(s)
> ==7938==    at 0x72A8F7: krb5_rc_io_fetch (rc_dfl.c:356)
> ==7938==    by 0x72ABE5: krb5_rc_dfl_recover_locked (rc_dfl.c:460)
> ==7938==    by 0x72AF19: krb5_rc_dfl_recover_or_init (rc_dfl.c:522)
> ==7938==    by 0x6F68B4: krb5_rc_recover_or_initialize (rcfns.c:44)
> ==7938==    by 0x6F1671: krb5_get_server_rcache (srv_rcache.c:115)
> ==7938==    by 0x6EC4A6: krb5_rd_req (rd_req.c:89)
> ... (my code which looks something like this:
> 
>     rc = krb5_auth_con_init(m_kcontext, &serverAuthContext);
> 
>     ...
> 
>     rc = krb5_rd_req(m_kcontext,
>                      &serverAuthContext,
>                      &ticket,
>                      NULL /* don't check SPN */,
>                      &ktab,
>                      NULL /* no options */,
>                      ret_ticket);
> 
> Where we're verifying a user ticket by requesting a service ticket for
> the local host to prevent the classic rogue KDC response attack.   In
> anycase, it appears krb5_rc_io_read doesn't protect against a short
> read:
> 
> In krb5_rc_io_fetch, we have:
> 
>     int len2;
>     unsigned int len;
>     krb5_error_code retval;
> 
>     rep->client = rep->server = 0;
> 
>     retval = krb5_rc_io_read(context, &t->d, (krb5_pointer) &len2,
>                              sizeof(len2));
>     if (retval)
>         return retval;
> 
>     if ((len2 <= 0) || (len2 >= maxlen))   // line 356
>         return KRB5_RC_IO_EOF;
> 
> And krb5_rc_io_read is:
> 
> krb5_error_code
> krb5_rc_io_read(krb5_context context, krb5_rc_iostuff *d, 
> krb5_pointer buf,
>                 unsigned int num)
> {
>     int count;
>     if ((count = read(d->fd, (char *) buf, num)) == -1)
>         switch(errno)
>         {
>         case EIO: return KRB5_RC_IO_IO;
>         case EBADF:
>         default:
>             krb5_set_error_message(context, KRB5_RC_IO_UNKNOWN,
>                                    "Can't read from replay cache: %s",
>                                    strerror(errno));
>             return KRB5_RC_IO_UNKNOWN;
>         }
>     if (count == 0)
>         return KRB5_RC_IO_EOF;
>     return 0;
> }
> 
> So there is no guarantee that we're are going to read 4 bytes.   I
> suspect in this case we read 1, 2 or 3, and valgrind 
> complained about some of the bytes in len2 not being 
> initialized (or fstat() didn't initialize st_size?).
> 
> Now I strongly suspect this was a side effect of running 
> within valgrind
> due to the performance issues introduced.    It only happened once
> during a 24 hour stress test of this code path, but I confess 
> to not having a deep understanding of what's happening at 
> this point in time.
> We could obviously pacify valgrind by initializing len2 to 0, 
> but I don't think that's necessarily the right thing to do... 
>  Is it possible
> the length value would be less than 4 bytes?   Any thoughts on whether
> this is a real-world problem?
> 
> mp
> 
> 
> _______________________________________________
> krbdev mailing list             krbdev at mit.edu
> https://mailman.mit.edu/mailman/listinfo/krbdev
>