This issue is caused by dnsmasq incorrectly handling the client's tag
information. It is not related to adding the opts item.
This is a necessary issue, and the steps to reproduce it are as follows:
1. Run dnsmasq with --dhcp-hostsfile
# dnsmasq --dhcp-hostsfile=/var/run/test_dhcp/host
--dhcp-range=192.168.0.0,192.168.0.240,infinite
# cat /var/run/test_dhcp/host
d0:c0:c4:66:25:81,host-192-168-0-159.dhcp-test,
192.168.0.159,set:port-cfc5b9f7-2760-42d6-899b-6817fe405721
2. Request a dynamic address for the client that comply with the host rule
# dhclient -cf /etc/dhcp/dhclient.conf -pf /tmp/eth1.pid -lf /tmp/eth1.lease
eth1
# cat /etc/dhcp/dhclient.conf
send dhcp-client-identifier " host-192-168-0-159.dhcp-test";
# ip link show eth1
2: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode
DEFAULT group default qlen 1000
link/ether d0:c0:c4:66:25:81 brd ff:ff:ff:ff:ff:ff
3. Send SIGHUP to dnsmasq
# kill -1 446255
In addition to the above issue, dnsmasq may also be related to a UAF problem.
Here is the patch I provided for fixing these issues.
>From 31b883b2bcaf70b74549128ded853f4942e5f6ae Mon Sep 17 00:00:00 2001
From: renmingshuai <renmingsh...@huawei.com>
Date: Thu, 1 Aug 2024 19:56:47 +0800
Subject: [PATCH] Fix a coredump and UAF caused by trying to free non-heap
objects.
There is an issue with the process of adding a netid in dhcp_deply for dnsmasq,
which will trigger coredump and UAF.
When processing DHCP requests, dnsmasq uses local variables such as known_id,
iface_id and cpewan_id, to store the client's tag information and add them to
the netid list.
Afterwards, dnsmasq will call find_config() to find a dhcp config and add the
netid list to the global daemon->dhcp_conf->netid list.
In addition, dnsmasq will also add the netid from other global variables to the
config->netid list, such as daemon->dhcp_match->netid
When dnsmasq receives the hup signal, it will call clear_dynamic_conf() to free
the netid in the config's netid list, which will trigger at least the following
three issues:
1. config->netid->list may point to a stack address (such as the address of the
local variable "known_id" mentioned above), and the content of this address may
be assigned other values.
Attempting to free this illegal address value will trigger coredump.
2. config->netid->list->net may point to a string constant (such as
known_id.net="knwon"), and attempting to free this address will also trigger
coredump.
3. The address pointed to by config->netid->list may be also added to other
global lists, such as daemon->dhcp_match->netid.
After the address is freed, it still exists in the daemon->dhcp_match list,
which may trigger UAF issues.
Calling dhcp_netid_creat() to allocate new memory for the client's tag
information can solve the above issues. These memories will be fully freed in
clear_rynamic_comf().
Signed-off-by: renmingshuai <renmingsh...@huawei.com>
---
src/dnsmasq.h | 1 +
src/option.c | 2 +-
src/rfc2131.c | 48 ++++++++++++++----------------------------------
3 files changed, 16 insertions(+), 35 deletions(-)
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index e455c3f..3376655 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1519,6 +1519,7 @@ char *parse_server(char *arg, struct server_details
*sdetails);
char *parse_server_addr(struct server_details *sdetails);
int parse_server_next(struct server_details *sdetails);
int option_read_dynfile(char *file, int flags);
+struct dhcp_netid *dhcp_netid_create(const char *net, struct dhcp_netid *next);
/* forward.c */
void reply_query(int fd, time_t now);
diff --git a/src/option.c b/src/option.c
index f4ff7c0..480ef5f 100644
--- a/src/option.c
+++ b/src/option.c
@@ -1314,7 +1314,7 @@ static char *set_prefix(char *arg)
return arg;
}
-static struct dhcp_netid *dhcp_netid_create(const char *net, struct dhcp_netid
*next)
+struct dhcp_netid *dhcp_netid_create(const char *net, struct dhcp_netid *next)
{
struct dhcp_netid *tt;
tt = opt_malloc(sizeof (struct dhcp_netid));
diff --git a/src/rfc2131.c b/src/rfc2131.c
index 68834ea..050d610 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -96,7 +96,6 @@ size_t dhcp_reply(struct dhcp_context *context, char
*iface_name, int int_index,
unsigned char *agent_id = NULL, *uuid = NULL;
unsigned char *emac = NULL;
int vendor_class_len = 0, emac_len = 0;
- struct dhcp_netid known_id, iface_id, cpewan_id;
struct dhcp_opt *o;
unsigned char pxe_uuid[17];
unsigned char *oui = NULL, *serial = NULL;
@@ -107,9 +106,7 @@ size_t dhcp_reply(struct dhcp_context *context, char
*iface_name, int int_index,
subnet_addr.s_addr = override.s_addr = 0;
/* set tag with name == interface */
- iface_id.net = iface_name;
- iface_id.next = NULL;
- netid = &iface_id;
+ netid = dhcp_netid_create(iface_name, NULL);
if (mess->op != BOOTREQUEST || mess->hlen > DHCP_CHADDR_MAX)
return 0;
@@ -173,9 +170,7 @@ size_t dhcp_reply(struct dhcp_context *context, char
*iface_name, int int_index,
the gateway id back. Note that the device class is
optional */
if (oui && serial)
{
- cpewan_id.net = "cpewan-id";
- cpewan_id.next = netid;
- netid = &cpewan_id;
+ netid = dhcp_netid_create("cpewan-id", netid);
}
break;
}
@@ -226,8 +221,7 @@ size_t dhcp_reply(struct dhcp_context *context, char
*iface_name, int int_index,
vendor->len == option_len(sopt) &&
memcmp(option_ptr(sopt, 0), vendor->data, vendor->len) == 0)
{
- vendor->netid.next = netid;
- netid = &vendor->netid;
+ netid = dhcp_netid_create(vendor->netid.net, netid);
}
}
}
@@ -263,8 +257,7 @@ size_t dhcp_reply(struct dhcp_context *context, char
*iface_name, int int_index,
(mac->hwaddr_type == mess->htype || mac->hwaddr_type == 0) &&
memcmp_masked(mac->hwaddr, mess->chaddr, mess->hlen, mac->mask))
{
- mac->netid.next = netid;
- netid = &mac->netid;
+ netid = dhcp_netid_create(mac->netid.net, netid);
}
/* Determine network for this packet. Our caller will have already linked
all the
@@ -453,8 +446,7 @@ size_t dhcp_reply(struct dhcp_context *context, char
*iface_name, int int_index,
if (match)
{
- o->netid->next = netid;
- netid = o->netid;
+ netid = dhcp_netid_create(o->netid->net, netid);
}
}
@@ -499,8 +491,7 @@ size_t dhcp_reply(struct dhcp_context *context, char
*iface_name, int int_index,
for (i = 0; i <= (option_len(opt) - vendor->len); i++)
if (memcmp(vendor->data, option_ptr(opt, i), vendor->len) == 0)
{
- vendor->netid.next = netid;
- netid = &vendor->netid;
+ netid = dhcp_netid_create(vendor->netid.net, netid);
break;
}
}
@@ -531,16 +522,12 @@ size_t dhcp_reply(struct dhcp_context *context, char
*iface_name, int int_index,
/* set "known" tag for known hosts */
if (config)
{
- known_id.net = "known";
- known_id.next = netid;
- netid = &known_id;
+ netid = dhcp_netid_create("known", netid);
}
else if (find_config(daemon->dhcp_conf, NULL, clid, clid_len,
mess->chaddr, mess->hlen, mess->htype, NULL,
run_tag_if(netid)))
{
- known_id.net = "known-othernet";
- known_id.next = netid;
- netid = &known_id;
+ netid = dhcp_netid_create("known-othernet", netid);
}
if (mess_type == 0 && !pxe)
@@ -580,16 +567,12 @@ size_t dhcp_reply(struct dhcp_context *context, char
*iface_name, int int_index,
{
memcpy(daemon->dhcp_buff2, mess->file, sizeof(mess->file));
daemon->dhcp_buff2[sizeof(mess->file) + 1] = 0; /* ensure zero term.
*/
- id.net = (char *)daemon->dhcp_buff2;
- id.next = netid;
- netid = &id;
+ netid = dhcp_netid_create((char *)daemon->dhcp_buff2, netid);
}
/* Add "bootp" as a tag to allow different options, address ranges etc
for BOOTP clients */
- bootp_id.net = "bootp";
- bootp_id.next = netid;
- netid = &bootp_id;
+ netid = dhcp_netid_create("bootp", netid);
tagif_netid = run_tag_if(netid);
@@ -634,8 +617,8 @@ size_t dhcp_reply(struct dhcp_context *context, char
*iface_name, int int_index,
message = _("wrong network");
else if (context->netid.net)
{
- context->netid.next = netid;
- tagif_netid = run_tag_if(&context->netid);
+ netid = dhcp_netid_create(context->netid.net, netid);
+ tagif_netid = run_tag_if(netid);
}
log_tags(tagif_netid, ntohl(mess->xid));
@@ -770,8 +753,7 @@ size_t dhcp_reply(struct dhcp_context *context, char
*iface_name, int int_index,
if (hostname_isequal(client_hostname, m->name) &&
(save == 0 || m->wildcard))
{
- m->netid->next = netid;
- netid = m->netid;
+ netid = dhcp_netid_create(m->netid->net, netid);
}
if (save != 0)
@@ -808,9 +790,7 @@ size_t dhcp_reply(struct dhcp_context *context, char
*iface_name, int int_index,
{
config = new;
/* set "known" tag for known hosts */
- known_id.net = "known";
- known_id.next = netid;
- netid = &known_id;
+ netid = dhcp_netid_create("known", netid);
}
}
}
--
2.33.0
_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
