From e3607d546d753a1831dbb532d3148b2cc653f8e6 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 13 Jul 2026 15:16:15 +0200 Subject: [PATCH] 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: 0897cda33b63 ("Add a constructor to create in-memory pools from an address range") --- src/libcharon/attributes/mem_pool.c | 14 ++++--- src/libcharon/tests/suites/test_mem_pool.c | 44 ++++++++++++++++++++++ 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/libcharon/attributes/mem_pool.c b/src/libcharon/attributes/mem_pool.c index a7d039448..1db5b3f26 100644 --- a/src/libcharon/attributes/mem_pool.c +++ b/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; } diff --git a/src/libcharon/tests/suites/test_mem_pool.c b/src/libcharon/tests/suites/test_mem_pool.c index ae10e3a36..35b7a4884 100644 --- a/src/libcharon/tests/suites/test_mem_pool.c +++ b/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); }