From: Dainis Jonitis <dainis.jonitis@ubnt.com>
Date: Mon, 26 Nov 2018 12:43:45 +0000 (+0200)
Subject: dhcpv4: DHCP pool size is off-by-one
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=39e11ed2d9de55a2661c21176b74263988e932d3;p=oweals%2Fodhcpd.git

dhcpv4: DHCP pool size is off-by-one

1. "limit" option should specify the size of dynamic pool. The dhcpv4_end
   includes the last valid pool address.
2. Also handle 7 bit host addresses when not directly specified in config file.
3. Make sure code does what documentation says and default 'start'/'limit'
   pool options to 100 and 150 respectively.

Signed-off-by: Dainis Jonitis <dainis.jonitis@ubnt.com>
Signed-off-by: Hans Dedecker <dedeckeh@gmail.com>
---

diff --git a/src/config.c b/src/config.c
index 7469731..47eb65f 100644
--- a/src/config.c
+++ b/src/config.c
@@ -32,6 +32,9 @@ struct config config = {.legacy = false, .main_dhcpv4 = false,
 			.dhcp_cb = NULL, .dhcp_statefile = NULL,
 			.log_level = LOG_WARNING};
 
+#define START_DEFAULT	100
+#define LIMIT_DEFAULT	150
+
 enum {
 	IFACE_ATTR_INTERFACE,
 	IFACE_ATTR_IFNAME,
@@ -210,6 +213,8 @@ static void set_interface_defaults(struct interface *iface)
 {
 	iface->learn_routes = 1;
 	iface->dhcpv4_leasetime = 43200;
+	iface->dhcpv4_start.s_addr = htonl(START_DEFAULT);
+	iface->dhcpv4_end.s_addr = htonl(START_DEFAULT + LIMIT_DEFAULT - 1);
 	iface->dhcpv6_assignall = true;
 	iface->dhcpv6_pd = true;
 	iface->dhcpv6_na = true;
@@ -500,14 +505,16 @@ int config_parse_interface(void *data, size_t len, const char *name, bool overwr
 
 	if ((c = tb[IFACE_ATTR_START])) {
 		iface->dhcpv4_start.s_addr = htonl(blobmsg_get_u32(c));
+		iface->dhcpv4_end.s_addr = htonl(ntohl(iface->dhcpv4_start.s_addr) +
+							LIMIT_DEFAULT - 1);
 
 		if (config.main_dhcpv4 && config.legacy)
 			iface->dhcpv4 = MODE_SERVER;
 	}
 
 	if ((c = tb[IFACE_ATTR_LIMIT]))
-		iface->dhcpv4_end.s_addr = htonl(
-				ntohl(iface->dhcpv4_start.s_addr) + blobmsg_get_u32(c));
+		iface->dhcpv4_end.s_addr = htonl(ntohl(iface->dhcpv4_start.s_addr) +
+							blobmsg_get_u32(c) - 1);
 
 	if ((c = tb[IFACE_ATTR_MASTER]))
 		iface->master = blobmsg_get_bool(c);
diff --git a/src/dhcpv4.c b/src/dhcpv4.c
index 370e1b9..679b793 100644
--- a/src/dhcpv4.c
+++ b/src/dhcpv4.c
@@ -263,18 +263,21 @@ static int setup_dhcpv4_addresses(struct interface *iface)
 	end = start = iface->dhcpv4_local.s_addr & iface->dhcpv4_mask.s_addr;
 
 	/* Auto allocate ranges */
-	if (ntohl(iface->dhcpv4_mask.s_addr) <= 0xffffff00) {
+	if (ntohl(iface->dhcpv4_mask.s_addr) <= 0xffffff00) {		/* /24, 150 of 256, [100..249] */
 		iface->dhcpv4_start_ip.s_addr = start | htonl(100);
-		iface->dhcpv4_end_ip.s_addr = end | htonl(250);
-	} else if (ntohl(iface->dhcpv4_mask.s_addr) <= 0xffffffc0) {
+		iface->dhcpv4_end_ip.s_addr = end | htonl(100 + 150 - 1);
+	} else if (ntohl(iface->dhcpv4_mask.s_addr) <= 0xffffff80) {    /* /25, 100 of 128, [20..119] */
+		iface->dhcpv4_start_ip.s_addr = start | htonl(20);
+		iface->dhcpv4_end_ip.s_addr = end | htonl(20 + 100 - 1);
+	} else if (ntohl(iface->dhcpv4_mask.s_addr) <= 0xffffffc0) {    /* /26, 50 of 64, [10..59] */
 		iface->dhcpv4_start_ip.s_addr = start | htonl(10);
-		iface->dhcpv4_end_ip.s_addr = end | htonl(60);
-	} else if (ntohl(iface->dhcpv4_mask.s_addr) <= 0xffffffe0) {
+		iface->dhcpv4_end_ip.s_addr = end | htonl(10 + 50 - 1);
+	} else if (ntohl(iface->dhcpv4_mask.s_addr) <= 0xffffffe0) {    /* /27, 20 of 32, [10..29] */
 		iface->dhcpv4_start_ip.s_addr = start | htonl(10);
-		iface->dhcpv4_end_ip.s_addr = end | htonl(30);
-	} else {
+		iface->dhcpv4_end_ip.s_addr = end | htonl(10 + 20 - 1);
+	} else {							/* /28, 10 of 16, [3..12] */
 		iface->dhcpv4_start_ip.s_addr = start | htonl(3);
-		iface->dhcpv4_end_ip.s_addr = end | htonl(12);
+		iface->dhcpv4_end_ip.s_addr = end | htonl(3 + 10 - 1);
 	}
 
 	return 0;