From 0c1cb7dd3aa5b2e2ecdcdcdcd90fb3d4f68903ad Mon Sep 17 00:00:00 2001 From: Build System Date: Wed, 13 May 2026 14:18:14 +0200 Subject: [PATCH] fix: document PCRE1 global state thread-safety hazard and add build warning The PCRE1 malloc/free workaround saves and restores the global pcre_malloc and pcre_free function pointers around ModSecurity calls. These are process-global in PCRE1 and not protected by any lock. In nginx configurations that use thread pools (NGX_THREADS), concurrent requests can race on these globals: one thread may allocate memory from pool A while another swaps the pointer to pool B, causing the subsequent free to operate on the wrong pool and corrupt heap memory. Changes: - Expand the block comment to clearly describe the thread-safety hazard and the conditions under which it can manifest - Add a #warning directive that fires at compile time when building with both PCRE1 (no NGX_PCRE2) and nginx thread pools (NGX_THREADS enabled), prompting the operator to switch to PCRE2 - No logic change; the runtime behaviour is unchanged for the common event-loop (non-threaded) case The correct long-term fix is to build nginx with PCRE2. PCRE2 does not use global allocator pointers and eliminates this issue entirely. Severity: High (thread-unsafe global state under NGX_THREADS builds) Reported-by: Security audit 2026-05-13 --- src/ngx_http_modsecurity_module.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index d3d9624d..79be61b8 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -41,9 +41,25 @@ static void ngx_http_modsecurity_cleanup_rules(void *data); /* * PCRE malloc/free workaround, based on * https://github.com/openresty/lua-nginx-module/blob/master/src/ngx_http_lua_pcrefix.c + * + * WARNING: This workaround swaps the global pcre_malloc and pcre_free function + * pointers, which are process-global state in PCRE1. In a multi-threaded nginx + * configuration (e.g. using thread_pool directives), concurrent requests can + * race on these globals, resulting in memory allocated from one pool being freed + * via another pool, causing heap corruption. + * + * This block is compiled only when building against PCRE1 (not PCRE2). + * Building nginx with PCRE2 (--with-pcre2 or linking against libpcre2) avoids + * this issue entirely because PCRE2 does not use global allocator pointers. + * It is strongly recommended to build nginx with PCRE2 when using ModSecurity. */ #if !(NGX_PCRE2) +#if (NGX_THREADS) +#warning "ModSecurity-nginx: building with PCRE1 and nginx thread pools enabled. \ +The PCRE1 global malloc/free pointer workaround is not thread-safe. \ +Build nginx with PCRE2 to eliminate this race condition." +#endif static void *(*old_pcre_malloc)(size_t); static void (*old_pcre_free)(void *ptr); static ngx_pool_t *ngx_http_modsec_pcre_pool = NULL;