static void
cbcp_recvreq(us, pckt, pcktlen)
cbcp_state *us;
u_char *pckt;
int pcktlen;
{
u_char type, opt_len, delay, addr_type;
char address[256];
int len = pcktlen;
address[0] = 0;
while (len >= 2) {
dbglog("length: %d", len);
GETCHAR(type, pckt);
GETCHAR(opt_len, pckt);
if (opt_len < 2 || opt_len > len)
break;
if (opt_len > 2)
GETCHAR(delay, pckt);
us->us_allowed |= (1 << type);
switch(type) {
case CB_CONF_NO:
dbglog("no callback allowed");
break;
case CB_CONF_USER:
dbglog("user callback allowed");
if (opt_len > 4) {
GETCHAR(addr_type, pckt);
memcpy(address, pckt, opt_len - 4);
address[opt_len - 4] = 0;
if (address[0])
dbglog("address: %s", address);
}
break;
case CB_CONF_ADMIN:
dbglog("user admin defined allowed");
break;
case CB_CONF_LIST:
break;
}
len -= opt_len;
}
if (len != 0) {
if (debug)
dbglog("cbcp_recvreq: malformed packet (%d bytes left)", len);
return;
}
cbcp_resp(us);
}
Now I draw your attention to the loop condition. while (len >= 2). Lets look at the router firmware code:
while (len) {
dbglog("length: %d", len);
GETCHAR(type, pckt);
GETCHAR(opt_len, pckt);
if (opt_len > 2)
GETCHAR(delay, pckt);
us->us_allowed |= (1 << type);
switch(type) {
case CB_CONF_NO:
dbglog("no callback allowed");
break;
case CB_CONF_USER:
dbglog("user callback allowed");
if (opt_len > 4) {
GETCHAR(addr_type, pckt);
memcpy(address, pckt, opt_len - 4);
address[opt_len - 4] = 0;
if (address[0])
dbglog("address: %s", address);
}
break;
case CB_CONF_ADMIN:
dbglog("user admin defined allowed");
break;
case CB_CONF_LIST:
break;
}
len -= opt_len;
}
What's going to happen if the opt_len's don't add up to the packet size? Lets think of when len is 1 and opt_len is 2. It'll wrap around to a non zero negative. And the loop will keep iterating.
Clearly buggy.
