[lwip-devel] [bug #28606] PPP: Possible bug in fsm.c:fsm_input()

[Iordan Neshev]

URL:
  <http://savannah.nongnu.org/bugs/?28606>

                 Summary: PPP: Possible bug in fsm.c:fsm_input()
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: iordan_neshev
            Submitted on: Thu 14 Jan 2010 12:48:41 PM GMT
                Category: None
                Severity: 3 - Normal
              Item Group: None
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: 
            lwIP version: CVS Head

    _______________________________________________________

Details:

This is a snippet of this function
void fsm_input(fsm *f, u_char *inpacket, int l)
{
  ...
  switch (code) {// CONFREQ, CONFACK, CONFNAK ...
  ....
  
      default:
      FSMDEBUG((LOG_INFO, "fsm_input(%s): default: \n", PROTO_NAME(f)));

      if( !f->callbacks->extcode ||
          !(*f->callbacks->extcode)(f, code, id, inp, len) ) {
        fsm_sdata(f, CODEREJ, ++f->id, inpacket, len + HEADERLEN);
      }

The code in the default section looks suspicious to me.
If I understand it right ->extcode is pointer to a default callback function
which should handle an incomming packet with code (described in fsm.h) which
could not be recognized. If this default handler was also unable to handle the
packet, then the packets is rejected by calling fsm_sdata(f, CODEREJ,
.....);.

Every PPP protocol can have such callback, but currently
only ipcp and lcp define one - ipcp_callbacks and lcp_callbacks
respectively.

Initialization:
f->callbacks = &ipcp_callbacks; for ipcp
f->callbacks = &lcp_callbacks;  for lcp
f->callbacks = NULL for all other ppp protocols

ipcp_callbacks->extcode = NULL;
lcp_callbacks->extcode = lcp_extcode()


Since extcode for ipcp is NULL, all IPCP packets with unknown code (i.e. not
handled by fsm_input) are immediately rejected 

Unknown LCP packets are passed to lcp_extcode(). If it too fails to recognize
the code, the packet is rejected as desribed.

Did I figured it out right?

If yes, then there is a possibility that
1. f->callbacks->extcode is evaluated when f->callbacks is NULL 
2.(*f->callbacks->extcode)(f, code, id, inp, len)
could be called when f->callbacks->extcode is NULL.

Both would lead to reading from undefined memory location and/or execution of
random code, which may be outside the memory space.

According to me the code should behave like that:

default:
{
  u8_t send_rej = 0;
  FSMDEBUG((LOG_INFO, "fsm_input(%s): default: \n", PROTO_NAME(f)));

  if (!f->callbacks)
  {
    send_rej = 1;
  }
  else
  {
    if( !f->callbacks->extcode) 
    {
      send_rej = 1;
    }
    else
    {
      if (!(*f->callbacks->extcode)(f, code, id, inp, len) )
      {
        send_rej = 1;
      }
    }
  }

  if (send_rej) {
    fsm_sdata(f, CODEREJ, ++f->id, inpacket, len + HEADERLEN);
  }
}






    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?28606>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/