Short read from krb5_rc_io_read

[Mike Patnode]

Unfortunately, I don't have 100% repro case even without len2 being
initialized.  maxlen was initialized by a call to fstat, so I think it's
OK (if the fstat failed, it's 0).   Also I saw the warning twice in the
same thread, which I expect was the first and second reference of len2
on that line.  I've repro'd it twice.   If I can get it a third time,
I'll initialize len2 and see if that fixes the complaint, but can you
tell me if the length value being read should be variable number of
bytes?   That's not what I would expect...

mp

> -----Original Message-----
> From: JC Ferguson [mailto:jc at f5.com] 
> Sent: Friday, March 14, 2008 10:17 AM
> To: Mike Patnode; Kerberos developers list
> Subject: RE: Short read from krb5_rc_io_read
> 
> 
> 
> 
> 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
> > 
>