dhcpc: fix potential invalid memory reads

- check for minimum packet length (44 bytes, which includes the fields we
  read next)
- pass the correct remaining buffer length to dhcpc_parseoptions()
- dhcpc_parseoptions(): ensure we never read past the end of the buffer
This commit is contained in:
Beat Küng 2021-06-29 13:39:16 +02:00 committed by Xiang Xiao
parent 21f48846dd
commit 7fdd751b7a

View File

@ -326,52 +326,98 @@ static uint8_t dhcpc_parseoptions(FAR struct dhcpc_state *presult,
/* Get subnet mask in network order */ /* Get subnet mask in network order */
memcpy(&presult->netmask.s_addr, optptr + 2, 4); if (optptr + 6 <= end)
{
memcpy(&presult->netmask.s_addr, optptr + 2, 4);
}
else
{
nerr("Packet too short (netmask missing)\n");
}
break; break;
case DHCP_OPTION_ROUTER: case DHCP_OPTION_ROUTER:
/* Get the default router address in network order */ /* Get the default router address in network order */
memcpy(&presult->default_router.s_addr, optptr + 2, 4); if (optptr + 6 <= end)
{
memcpy(&presult->default_router.s_addr, optptr + 2, 4);
}
else
{
nerr("Packet too short (router address missing)\n");
}
break; break;
case DHCP_OPTION_DNS_SERVER: case DHCP_OPTION_DNS_SERVER:
/* Get the DNS server address in network order */ /* Get the DNS server address in network order */
memcpy(&presult->dnsaddr.s_addr, optptr + 2, 4); if (optptr + 6 <= end)
{
memcpy(&presult->dnsaddr.s_addr, optptr + 2, 4);
}
else
{
nerr("Packet too short (DNS address missing)\n");
}
break; break;
case DHCP_OPTION_MSG_TYPE: case DHCP_OPTION_MSG_TYPE:
/* Get message type */ /* Get message type */
type = *(optptr + 2); if (optptr + 3 <= end)
{
type = *(optptr + 2);
}
else
{
nerr("Packet too short (type missing)\n");
}
break; break;
case DHCP_OPTION_SERVER_ID: case DHCP_OPTION_SERVER_ID:
/* Get server address in network order */ /* Get server address in network order */
memcpy(&presult->serverid.s_addr, optptr + 2, 4); if (optptr + 6 <= end)
{
memcpy(&presult->serverid.s_addr, optptr + 2, 4);
}
else
{
nerr("Packet too short (server address missing)\n");
}
break; break;
case DHCP_OPTION_LEASE_TIME: case DHCP_OPTION_LEASE_TIME:
{
/* Get lease time (in seconds) in host order */ /* Get lease time (in seconds) in host order */
uint16_t tmp[2]; if (optptr + 6 <= end)
memcpy(tmp, optptr + 2, 4); {
presult->lease_time = ((uint32_t)ntohs(tmp[0])) << 16 | uint16_t tmp[2];
(uint32_t)ntohs(tmp[1]); memcpy(tmp, optptr + 2, 4);
} presult->lease_time = ((uint32_t)ntohs(tmp[0])) << 16 |
(uint32_t)ntohs(tmp[1]);
}
else
{
nerr("Packet too short (lease time missing)\n");
}
break; break;
case DHCP_OPTION_END: case DHCP_OPTION_END:
return type; return type;
} }
if (optptr + 1 >= end)
{
break;
}
optptr += optptr[1] + 2; optptr += optptr[1] + 2;
} }
@ -385,13 +431,15 @@ static uint8_t dhcpc_parseoptions(FAR struct dhcpc_state *presult,
static uint8_t dhcpc_parsemsg(FAR struct dhcpc_state_s *pdhcpc, int buflen, static uint8_t dhcpc_parsemsg(FAR struct dhcpc_state_s *pdhcpc, int buflen,
FAR struct dhcpc_state *presult) FAR struct dhcpc_state *presult)
{ {
if (pdhcpc->packet.op == DHCP_REPLY && if (buflen >= 44 && pdhcpc->packet.op == DHCP_REPLY &&
memcmp(pdhcpc->packet.xid, xid, sizeof(xid)) == 0 && memcmp(pdhcpc->packet.xid, xid, sizeof(xid)) == 0 &&
memcmp(pdhcpc->packet.chaddr, memcmp(pdhcpc->packet.chaddr,
pdhcpc->ds_macaddr, pdhcpc->ds_maclen) == 0) pdhcpc->ds_macaddr, pdhcpc->ds_maclen) == 0)
{ {
memcpy(&presult->ipaddr.s_addr, pdhcpc->packet.yiaddr, 4); memcpy(&presult->ipaddr.s_addr, pdhcpc->packet.yiaddr, 4);
return dhcpc_parseoptions(presult, &pdhcpc->packet.options[4], buflen); return dhcpc_parseoptions(presult, &pdhcpc->packet.options[4],
buflen -
(offsetof(struct dhcp_msg, options) + 4));
} }
return 0; return 0;