dhcp.c: handle_packet() calls get_(raw|cooked)_packet(), which returns a
signed value where values <0 are errors and >= 0 are lengths. Convert to an unsigned length value if the return is a length. Further, there is a real bug if get_(raw|cooked)_packet() returns an error. handle_packet() should return rather than continuing to validate the packet. The packet validation will almost surely fail, and the negative values of len are constrained to [-1,-2], and the values are determined by errors that are hard to control, so it is extremely unlikely that there are any security issues with this bug. The fix is trivial; the obviously-missing return statement bails out when there's a problem fetching a packet and ndhc immediately goes back to listening for another packet.
This commit is contained in:
parent
85fcc1e8f0
commit
e50bd431d6
12
ndhc/dhcp.c
12
ndhc/dhcp.c
@ -495,7 +495,7 @@ void set_listen_none(struct client_state_t *cs)
|
|||||||
change_listen_mode(cs, LM_NONE);
|
change_listen_mode(cs, LM_NONE);
|
||||||
}
|
}
|
||||||
|
|
||||||
static int validate_dhcp_packet(struct client_state_t *cs, int len,
|
static int validate_dhcp_packet(struct client_state_t *cs, size_t len,
|
||||||
struct dhcpmsg *packet, uint8_t *msgtype)
|
struct dhcpmsg *packet, uint8_t *msgtype)
|
||||||
{
|
{
|
||||||
if (len < sizeof *packet - sizeof packet->options) {
|
if (len < sizeof *packet - sizeof packet->options) {
|
||||||
@ -531,22 +531,22 @@ static int validate_dhcp_packet(struct client_state_t *cs, int len,
|
|||||||
void handle_packet(struct client_state_t *cs)
|
void handle_packet(struct client_state_t *cs)
|
||||||
{
|
{
|
||||||
uint8_t msgtype;
|
uint8_t msgtype;
|
||||||
int len;
|
|
||||||
struct dhcpmsg packet;
|
struct dhcpmsg packet;
|
||||||
|
|
||||||
if (cs->listenMode == LM_NONE)
|
if (cs->listenMode == LM_NONE)
|
||||||
return;
|
return;
|
||||||
len = cs->listenMode == LM_RAW ?
|
int r = cs->listenMode == LM_RAW ?
|
||||||
get_raw_packet(cs, &packet) : get_cooked_packet(&packet, cs->listenFd);
|
get_raw_packet(cs, &packet) : get_cooked_packet(&packet, cs->listenFd);
|
||||||
|
if (r < 0) {
|
||||||
if (len < 0) {
|
|
||||||
// Transient issue handled by packet collection functions.
|
// Transient issue handled by packet collection functions.
|
||||||
if (len == -2 || (len == -1 && errno == EINTR))
|
if (r == -2 || (r == -1 && errno == EINTR))
|
||||||
return;
|
return;
|
||||||
log_error("Error reading from listening socket: %s. Reopening.",
|
log_error("Error reading from listening socket: %s. Reopening.",
|
||||||
strerror(errno));
|
strerror(errno));
|
||||||
change_listen_mode(cs, cs->listenMode);
|
change_listen_mode(cs, cs->listenMode);
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
|
size_t len = (size_t)r;
|
||||||
|
|
||||||
if (!validate_dhcp_packet(cs, len, &packet, &msgtype))
|
if (!validate_dhcp_packet(cs, len, &packet, &msgtype))
|
||||||
return;
|
return;
|
||||||
|
Loading…
Reference in New Issue
Block a user