Short read from krb5_rc_io_read

[Mike Patnode]

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