Thu 14 Jan 2010 12:48:41 PM UTC, original submission:
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);
}
}
|