dhcp: fix parse error with chained pbfus

If a chained pbuf starts with DHCP_OPTION_PAD, an overflow check
triggers and the packet is ignored.

Fix this by changing the way the offset is increased for PAD.
Also ignore a packet that is missing the END option.

Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
This commit is contained in:
Simon Goldschmidt 2018-07-02 20:25:42 +02:00
parent cc8995823a
commit 19a929f5fb

View File

@ -1574,7 +1574,6 @@ again:
/* special option: no len encoded */ /* special option: no len encoded */
decode_len = len = 0; decode_len = len = 0;
/* will be increased below */ /* will be increased below */
offset--;
break; break;
case (DHCP_OPTION_SUBNET_MASK): case (DHCP_OPTION_SUBNET_MASK):
LWIP_ERROR("len == 4", len == 4, return ERR_VAL;); LWIP_ERROR("len == 4", len == 4, return ERR_VAL;);
@ -1639,44 +1638,48 @@ again:
op, len, q, val_offset); op, len, q, val_offset);
break; break;
} }
if (offset + len + 2 > 0xFFFF) { if (op == DHCP_OPTION_PAD) {
/* overflow */ offset++;
return ERR_BUF; } else {
} if (offset + len + 2 > 0xFFFF) {
offset = (u16_t)(offset + len + 2); /* overflow */
if (decode_len > 0) { return ERR_BUF;
u32_t value = 0; }
u16_t copy_len; offset = (u16_t)(offset + len + 2);
if (decode_len > 0) {
u32_t value = 0;
u16_t copy_len;
decode_next: decode_next:
LWIP_ASSERT("check decode_idx", decode_idx >= 0 && decode_idx < DHCP_OPTION_IDX_MAX); LWIP_ASSERT("check decode_idx", decode_idx >= 0 && decode_idx < DHCP_OPTION_IDX_MAX);
if (!dhcp_option_given(dhcp, decode_idx)) { if (!dhcp_option_given(dhcp, decode_idx)) {
copy_len = LWIP_MIN(decode_len, 4); copy_len = LWIP_MIN(decode_len, 4);
if (pbuf_copy_partial(q, &value, copy_len, val_offset) != copy_len) { if (pbuf_copy_partial(q, &value, copy_len, val_offset) != copy_len) {
return ERR_BUF;
}
if (decode_len > 4) {
/* decode more than one u32_t */
u16_t next_val_offset;
LWIP_ERROR("decode_len %% 4 == 0", decode_len % 4 == 0, return ERR_VAL;);
dhcp_got_option(dhcp, decode_idx);
dhcp_set_option_value(dhcp, decode_idx, lwip_htonl(value));
decode_len = (u8_t)(decode_len - 4);
next_val_offset = (u16_t)(val_offset + 4);
if (next_val_offset < val_offset) {
/* overflow */
return ERR_BUF; return ERR_BUF;
} }
val_offset = next_val_offset; if (decode_len > 4) {
decode_idx++; /* decode more than one u32_t */
goto decode_next; u16_t next_val_offset;
} else if (decode_len == 4) { LWIP_ERROR("decode_len %% 4 == 0", decode_len % 4 == 0, return ERR_VAL;);
value = lwip_ntohl(value); dhcp_got_option(dhcp, decode_idx);
} else { dhcp_set_option_value(dhcp, decode_idx, lwip_htonl(value));
LWIP_ERROR("invalid decode_len", decode_len == 1, return ERR_VAL;); decode_len = (u8_t)(decode_len - 4);
value = ((u8_t *)&value)[0]; next_val_offset = (u16_t)(val_offset + 4);
if (next_val_offset < val_offset) {
/* overflow */
return ERR_BUF;
}
val_offset = next_val_offset;
decode_idx++;
goto decode_next;
} else if (decode_len == 4) {
value = lwip_ntohl(value);
} else {
LWIP_ERROR("invalid decode_len", decode_len == 1, return ERR_VAL;);
value = ((u8_t *)&value)[0];
}
dhcp_got_option(dhcp, decode_idx);
dhcp_set_option_value(dhcp, decode_idx, value);
} }
dhcp_got_option(dhcp, decode_idx);
dhcp_set_option_value(dhcp, decode_idx, value);
} }
} }
if (offset >= q->len) { if (offset >= q->len) {
@ -1688,7 +1691,7 @@ decode_next:
options = (u8_t *)q->payload; options = (u8_t *)q->payload;
} else { } else {
/* We've run out of bytes, probably no end marker. Don't proceed. */ /* We've run out of bytes, probably no end marker. Don't proceed. */
break; return ERR_BUF;
} }
} }
} }