| From 58d397ba74873384aee449690a9070bacd5676fa Mon Sep 17 00:00:00 2001 |
| From: Colin Wee <cwee@tesla.com> |
| Date: Thu, 28 Jan 2021 19:39:14 +0100 |
| Subject: [PATCH] gdhcp: Avoid reading invalid data in dhcp_get_option |
| |
| Signed-off-by: Peter Korsgaard <peter@korsgaard.com> |
| --- |
| gdhcp/client.c | 20 +++++++++++--------- |
| gdhcp/common.c | 24 +++++++++++++++++++----- |
| gdhcp/common.h | 2 +- |
| gdhcp/server.c | 12 +++++++----- |
| 4 files changed, 38 insertions(+), 20 deletions(-) |
| |
| diff --git a/gdhcp/client.c b/gdhcp/client.c |
| index 09dfe5ec..6a5613e7 100644 |
| --- a/gdhcp/client.c |
| +++ b/gdhcp/client.c |
| @@ -1629,12 +1629,12 @@ static void start_request(GDHCPClient *dhcp_client) |
| NULL); |
| } |
| |
| -static uint32_t get_lease(struct dhcp_packet *packet) |
| +static uint32_t get_lease(struct dhcp_packet *packet, uint16_t packet_len) |
| { |
| uint8_t *option; |
| uint32_t lease_seconds; |
| |
| - option = dhcp_get_option(packet, DHCP_LEASE_TIME); |
| + option = dhcp_get_option(packet, packet_len, DHCP_LEASE_TIME); |
| if (!option) |
| return 3600; |
| |
| @@ -2226,7 +2226,8 @@ static void get_dhcpv6_request(GDHCPClient *dhcp_client, |
| } |
| } |
| |
| -static void get_request(GDHCPClient *dhcp_client, struct dhcp_packet *packet) |
| +static void get_request(GDHCPClient *dhcp_client, struct dhcp_packet *packet, |
| + uint16_t packet_len) |
| { |
| GDHCPOptionType type; |
| GList *list, *value_list; |
| @@ -2237,7 +2238,7 @@ static void get_request(GDHCPClient *dhcp_client, struct dhcp_packet *packet) |
| for (list = dhcp_client->request_list; list; list = list->next) { |
| code = (uint8_t) GPOINTER_TO_INT(list->data); |
| |
| - option = dhcp_get_option(packet, code); |
| + option = dhcp_get_option(packet, packet_len, code); |
| if (!option) { |
| g_hash_table_remove(dhcp_client->code_value_hash, |
| GINT_TO_POINTER((int) code)); |
| @@ -2297,6 +2298,7 @@ static gboolean listener_event(GIOChannel *channel, GIOCondition condition, |
| re = dhcp_recv_l2_packet(&packet, |
| dhcp_client->listener_sockfd, |
| &dst_addr); |
| + pkt_len = (uint16_t)(unsigned int)re; |
| xid = packet.xid; |
| } else if (dhcp_client->listen_mode == L3) { |
| if (dhcp_client->type == G_DHCP_IPV6) { |
| @@ -2361,7 +2363,7 @@ static gboolean listener_event(GIOChannel *channel, GIOCondition condition, |
| dhcp_client->status_code = status; |
| } |
| } else { |
| - message_type = dhcp_get_option(&packet, DHCP_MESSAGE_TYPE); |
| + message_type = dhcp_get_option(&packet, pkt_len, DHCP_MESSAGE_TYPE); |
| if (!message_type) |
| return TRUE; |
| } |
| @@ -2378,7 +2380,7 @@ static gboolean listener_event(GIOChannel *channel, GIOCondition condition, |
| dhcp_client->timeout = 0; |
| dhcp_client->retry_times = 0; |
| |
| - option = dhcp_get_option(&packet, DHCP_SERVER_ID); |
| + option = dhcp_get_option(&packet, pkt_len, DHCP_SERVER_ID); |
| dhcp_client->server_ip = get_be32(option); |
| dhcp_client->requested_ip = ntohl(packet.yiaddr); |
| |
| @@ -2428,9 +2430,9 @@ static gboolean listener_event(GIOChannel *channel, GIOCondition condition, |
| |
| remove_timeouts(dhcp_client); |
| |
| - dhcp_client->lease_seconds = get_lease(&packet); |
| + dhcp_client->lease_seconds = get_lease(&packet, pkt_len); |
| |
| - get_request(dhcp_client, &packet); |
| + get_request(dhcp_client, &packet, pkt_len); |
| |
| switch_listening_mode(dhcp_client, L_NONE); |
| |
| @@ -2438,7 +2440,7 @@ static gboolean listener_event(GIOChannel *channel, GIOCondition condition, |
| dhcp_client->assigned_ip = get_ip(packet.yiaddr); |
| |
| if (dhcp_client->state == REBOOTING) { |
| - option = dhcp_get_option(&packet, |
| + option = dhcp_get_option(&packet, pkt_len, |
| DHCP_SERVER_ID); |
| dhcp_client->server_ip = get_be32(option); |
| } |
| diff --git a/gdhcp/common.c b/gdhcp/common.c |
| index 1d667d17..c8916aa8 100644 |
| --- a/gdhcp/common.c |
| +++ b/gdhcp/common.c |
| @@ -73,18 +73,21 @@ GDHCPOptionType dhcp_get_code_type(uint8_t code) |
| return OPTION_UNKNOWN; |
| } |
| |
| -uint8_t *dhcp_get_option(struct dhcp_packet *packet, int code) |
| +uint8_t *dhcp_get_option(struct dhcp_packet *packet, uint16_t packet_len, int code) |
| { |
| int len, rem; |
| - uint8_t *optionptr; |
| + uint8_t *optionptr, *options_end; |
| + size_t options_len; |
| uint8_t overload = 0; |
| |
| /* option bytes: [code][len][data1][data2]..[dataLEN] */ |
| optionptr = packet->options; |
| rem = sizeof(packet->options); |
| + options_len = packet_len - (sizeof(*packet) - sizeof(packet->options)); |
| + options_end = optionptr + options_len - 1; |
| |
| while (1) { |
| - if (rem <= 0) |
| + if ((rem <= 0) && (optionptr + OPT_CODE > options_end)) |
| /* Bad packet, malformed option field */ |
| return NULL; |
| |
| @@ -115,14 +118,25 @@ uint8_t *dhcp_get_option(struct dhcp_packet *packet, int code) |
| break; |
| } |
| |
| + if (optionptr + OPT_LEN > options_end) { |
| + /* bad packet, would read length field from OOB */ |
| + return NULL; |
| + } |
| + |
| len = 2 + optionptr[OPT_LEN]; |
| |
| rem -= len; |
| if (rem < 0) |
| continue; /* complain and return NULL */ |
| |
| - if (optionptr[OPT_CODE] == code) |
| - return optionptr + OPT_DATA; |
| + if (optionptr[OPT_CODE] == code) { |
| + if (optionptr + len > options_end) { |
| + /* bad packet, option length points OOB */ |
| + return NULL; |
| + } else { |
| + return optionptr + OPT_DATA; |
| + } |
| + } |
| |
| if (optionptr[OPT_CODE] == DHCP_OPTION_OVERLOAD) |
| overload |= optionptr[OPT_DATA]; |
| diff --git a/gdhcp/common.h b/gdhcp/common.h |
| index 9660231c..8f63fd75 100644 |
| --- a/gdhcp/common.h |
| +++ b/gdhcp/common.h |
| @@ -179,7 +179,7 @@ struct in6_pktinfo { |
| }; |
| #endif |
| |
| -uint8_t *dhcp_get_option(struct dhcp_packet *packet, int code); |
| +uint8_t *dhcp_get_option(struct dhcp_packet *packet, uint16_t packet_len, int code); |
| uint8_t *dhcpv6_get_option(struct dhcpv6_packet *packet, uint16_t pkt_len, |
| int code, uint16_t *option_len, int *option_count); |
| uint8_t *dhcpv6_get_sub_option(unsigned char *option, uint16_t max_len, |
| diff --git a/gdhcp/server.c b/gdhcp/server.c |
| index 85405f19..52ea2a55 100644 |
| --- a/gdhcp/server.c |
| +++ b/gdhcp/server.c |
| @@ -413,7 +413,7 @@ error: |
| } |
| |
| |
| -static uint8_t check_packet_type(struct dhcp_packet *packet) |
| +static uint8_t check_packet_type(struct dhcp_packet *packet, uint16_t packet_len) |
| { |
| uint8_t *type; |
| |
| @@ -423,7 +423,7 @@ static uint8_t check_packet_type(struct dhcp_packet *packet) |
| if (packet->op != BOOTREQUEST) |
| return 0; |
| |
| - type = dhcp_get_option(packet, DHCP_MESSAGE_TYPE); |
| + type = dhcp_get_option(packet, packet_len, DHCP_MESSAGE_TYPE); |
| |
| if (!type) |
| return 0; |
| @@ -651,6 +651,7 @@ static gboolean listener_event(GIOChannel *channel, GIOCondition condition, |
| struct dhcp_lease *lease; |
| uint32_t requested_nip = 0; |
| uint8_t type, *server_id_option, *request_ip_option; |
| + uint16_t packet_len; |
| int re; |
| |
| if (condition & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) { |
| @@ -661,12 +662,13 @@ static gboolean listener_event(GIOChannel *channel, GIOCondition condition, |
| re = dhcp_recv_l3_packet(&packet, dhcp_server->listener_sockfd); |
| if (re < 0) |
| return TRUE; |
| + packet_len = (uint16_t)(unsigned int)re; |
| |
| - type = check_packet_type(&packet); |
| + type = check_packet_type(&packet, packet_len); |
| if (type == 0) |
| return TRUE; |
| |
| - server_id_option = dhcp_get_option(&packet, DHCP_SERVER_ID); |
| + server_id_option = dhcp_get_option(&packet, packet_len, DHCP_SERVER_ID); |
| if (server_id_option) { |
| uint32_t server_nid = |
| get_unaligned((const uint32_t *) server_id_option); |
| @@ -675,7 +677,7 @@ static gboolean listener_event(GIOChannel *channel, GIOCondition condition, |
| return TRUE; |
| } |
| |
| - request_ip_option = dhcp_get_option(&packet, DHCP_REQUESTED_IP); |
| + request_ip_option = dhcp_get_option(&packet, packet_len, DHCP_REQUESTED_IP); |
| if (request_ip_option) |
| requested_nip = get_be32(request_ip_option); |
| |
| -- |
| 2.20.1 |
| |