mem-pool: Enforce size limit also for range-based pools and fix overflow

Due to the overflow when mapping addresses to offsets, no addresses could
get released when the pool was defined in a way that the last address
assignable is 255.255.255.255 (probably never the case in practice).

While not really useful in practice, the range 0.0.0.0-255.255.255.255
resulted in an empty pool (`size = 0xffffffff + 1`), which triggers the
%config like behavior.  Since the implementation currently uses signed
integers throughout, we make sure the largest assignable offset is
INT_MAX.

The two off-by-one fixes (`>= pool->size`) were never an issue in practice
because `get_new()` has a guard that caps the offset at the size and
released addresses originally came from the pool so are always in range.

Fixes: 0897cda33b ("Add a constructor to create in-memory pools from an address range")

src/libcharon/attributes/mem_pool.c

@@ -161,7 +161,7 @@ static host_t *apply_offset(host_t *base, int offset)
 static host_t* offset2host(private_mem_pool_t *pool, int offset)
 {
 	offset--;
-	if (offset > pool->size)
+	if (offset >= pool->size)
 	{
 		return NULL;
 	}
@@ -194,7 +194,7 @@ static int host2offset(private_mem_pool_t *pool, host_t *addr)
 	}
 	hosti = ntohl(*(uint32_t*)(host.ptr));
 	basei = ntohl(*(uint32_t*)(base.ptr));
-	if (hosti > basei + pool->size)
+	if (hosti - basei >= pool->size)
 	{
 		return -1;
 	}
@@ -742,11 +742,15 @@ mem_pool_t *mem_pool_create_range(char *name, host_t *from, host_t *to)
 		DBG1(DBG_CFG, "IP address range too large: %H-%H", from, to);
 		return NULL;
 	}
-	this = create_generic(name);
-	this->base = from->clone(from);
 	diff = untoh32(toaddr.ptr + toaddr.len - sizeof(diff)) -
 		   untoh32(fromaddr.ptr + fromaddr.len - sizeof(diff));
+	if (diff >= (1U << POOL_LIMIT) - 1)
+	{
+		DBG1(DBG_CFG, "IP address range too large: %H-%H", from, to);
+		return NULL;
+	}
+	this = create_generic(name);
+	this->base = from->clone(from);
 	this->size = diff + 1;
-
 	return &this->public;
 }

src/libcharon/tests/suites/test_mem_pool.c

@@ -63,6 +63,20 @@ static void assert_acquire(mem_pool_t *pool, char *requested, char *expected,
 	id->destroy(id);
 }
 
+static void assert_release(mem_pool_t *pool, char *released)
+{
+	identification_t *id;
+	host_t *rel;
+
+	id = identification_create_from_string("tester");
+	rel = host_create_from_string(released, 0);
+
+	ck_assert(pool->release_address(pool, rel, id));
+
+	rel->destroy(rel);
+	id->destroy(id);
+}
+
 static void assert_acquires_new(mem_pool_t *pool, char *pattern, int first)
 {
 	char expected[16];
@@ -222,6 +236,28 @@ START_TEST(test_range)
 	assert_acquires_new(pool, "192.168.0.%d", 10);
 	pool->destroy(pool);
 
+	from->destroy(from);
+	from = host_create_from_string("255.255.255.254", 0);
+	to->destroy(to);
+	to = host_create_from_string("255.255.255.255", 0);
+	pool = mem_pool_create_range("test", from, to);
+	ck_assert_int_eq(2, pool->get_size(pool));
+	assert_base(pool, "255.255.255.254");
+	assert_acquires_new(pool, "255.255.255.%d", 254);
+	assert_release(pool, "255.255.255.255");
+	ck_assert_int_eq(1, pool->get_online(pool));
+	assert_acquire(pool, "0.0.0.0", "255.255.255.255", MEM_POOL_EXISTING);
+	ck_assert_int_eq(2, pool->get_online(pool));
+	pool->destroy(pool);
+
+	/* this range is too large */
+	from->destroy(from);
+	from = host_create_from_string("0.0.0.0", 0);
+	to->destroy(to);
+	to = host_create_from_string("255.255.255.255", 0);
+	pool = mem_pool_create_range("test", from, to);
+	ck_assert(!pool);
+
 	from->destroy(from);
 	from = host_create_from_string("fec::1", 0);
 	to->destroy(to);
@@ -229,6 +265,14 @@ START_TEST(test_range)
 	pool = mem_pool_create_range("test", from, to);
 	ck_assert(!pool);
 
+	/* this range is too large */
+	from->destroy(from);
+	from = host_create_from_string("fec::", 0);
+	to->destroy(to);
+	to = host_create_from_string("fec::ffff:ffff", 0);
+	pool = mem_pool_create_range("test", from, to);
+	ck_assert(!pool);
+
 	from->destroy(from);
 	to->destroy(to);
 }