| From 591ed1e90503817938ccf5f127e677a8dd48b6d8 Mon Sep 17 00:00:00 2001 |
| From: Simon Kelley <simon@thekelleys.org.uk> |
| Date: Mon, 11 Jul 2016 18:18:42 +0100 |
| Subject: [PATCH] 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. |
| |
| src/rfc2131.c | 5 +++-- |
| 1 file changed, 3 insertions(+), 2 deletions(-) |
| |
| diff --git a/src/rfc2131.c b/src/rfc2131.c |
| index b7c167e..8b99d4b 100644 |
| |
| |
| @@ -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. */ |
| unsigned char *sopt; |
| 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) |
| { |
| end -= total; |
| @@ -1606,7 +1607,7 @@ static unsigned char *option_find1(unsigned char *p, unsigned char *end, int opt |
| { |
| while (1) |
| { |
| - if (p > end) |
| + if (p >= end) |
| return NULL; |
| else if (*p == OPTION_END) |
| return opt == OPTION_END ? p : NULL; |
| -- |
| 2.9.3 |
| |