Skip to content

fix: free intervention strings on mcf NULL early return path#379

Open
eilandert wants to merge 1 commit into
owasp-modsecurity:masterfrom
eilandert:fix/mcf-null-early-return-leak
Open

fix: free intervention strings on mcf NULL early return path#379
eilandert wants to merge 1 commit into
owasp-modsecurity:masterfrom
eilandert:fix/mcf-null-early-return-leak

Conversation

@eilandert
Copy link
Copy Markdown

When msc_intervention() succeeds and fills intervention.log and/or intervention.url, then ngx_http_get_module_loc_conf() returns NULL (unexpected misconfiguration), the function returned immediately with NGX_HTTP_INTERNAL_SERVER_ERROR without freeing either string.

Free both intervention.log and intervention.url before the early return to prevent the leak on this error path.

When msc_intervention() succeeds and fills intervention.log and/or
intervention.url, then ngx_http_get_module_loc_conf() returns NULL
(unexpected misconfiguration), the function returned immediately with
NGX_HTTP_INTERNAL_SERVER_ERROR without freeing either string.

Free both intervention.log and intervention.url before the early
return to prevent the leak on this error path.

Severity: Medium
Reported-by: Security audit 2026-05-13
@sonarqubecloud
Copy link
Copy Markdown

@airween
Copy link
Copy Markdown
Member

airween commented May 14, 2026

Hi @eilandert,

thanks for this PR too - here my question is the same: is it possible to add some tests?

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a memory leak in the Nginx ModSecurity connector by ensuring ModSecurity-provided intervention strings are freed when the request’s module loc-conf unexpectedly resolves to NULL.

Changes:

  • Free intervention.log and intervention.url before returning NGX_HTTP_INTERNAL_SERVER_ERROR on the mcf == NULL early-return path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +166 to +171
if (intervention.log != NULL) {
free(intervention.log);
}
if (intervention.url != NULL) {
free(intervention.url);
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants