Commit 591ed1e9 authored by Simon Kelley's avatar Simon Kelley

Fix bad behaviour with some DHCP option arrangements.

The check that there's enough space to store the DHCP agent-id
at the end of the packet could succeed when it should fail
if the END option is in either of the oprion-overload areas.
That could overwrite legit options in the request and cause
bad behaviour. It's highly unlikely that any sane DHCP client
would trigger this bug, and it's never been seen, but this
fixes the problem.

Also fix off-by-one in bounds checking of option processing.
Worst case scenario on that is a read one byte beyond the
end off a buffer with a crafted packet, and maybe therefore
a SIGV crash if the memory after the buffer is not mapped.

Thanks to Timothy Becker for spotting these.
parent 907efeb2
...@@ -186,7 +186,8 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, ...@@ -186,7 +186,8 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
be enough free space at the end of the packet to copy the option. */ be enough free space at the end of the packet to copy the option. */
unsigned char *sopt; unsigned char *sopt;
unsigned int total = option_len(opt) + 2; unsigned int total = option_len(opt) + 2;
unsigned char *last_opt = option_find(mess, sz, OPTION_END, 0); unsigned char *last_opt = option_find1(&mess->options[0] + sizeof(u32), ((unsigned char *)mess) + sz,
OPTION_END, 0);
if (last_opt && last_opt < end - total) if (last_opt && last_opt < end - total)
{ {
end -= total; end -= total;
...@@ -1606,7 +1607,7 @@ static unsigned char *option_find1(unsigned char *p, unsigned char *end, int opt ...@@ -1606,7 +1607,7 @@ static unsigned char *option_find1(unsigned char *p, unsigned char *end, int opt
{ {
while (1) while (1)
{ {
if (p > end) if (p >= end)
return NULL; return NULL;
else if (*p == OPTION_END) else if (*p == OPTION_END)
return opt == OPTION_END ? p : NULL; return opt == OPTION_END ? p : NULL;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment