Skip to content

fix: nullptr dereference in seclang scanner#3543

Open
airween wants to merge 2 commits intoowasp-modsecurity:v3/masterfrom
airween:v3/parserfix
Open

fix: nullptr dereference in seclang scanner#3543
airween wants to merge 2 commits intoowasp-modsecurity:v3/masterfrom
airween:v3/parserfix

Conversation

@airween
Copy link
Copy Markdown
Member

@airween airween commented Apr 12, 2026

what

This PR fixes a possible null pointer dereference in seclang-scanner and unhandled exception in rule parser.

why

There are two bugs reported, received in email from @fumfel and his team. Also they provided the fixes.

references

The original report:

Root Cause
----------

The flex scanner rules use the regex [ \t]+ to match whitespace between
directive names and their arguments. This regex matches both spaces and
tabs. However, the action code uses strchr(yytext, ' ') to find the
whitespace boundary, which searches only for space (' '), not tab
('\t').

When a configuration directive uses a tab as the separator (e.g.,
"SecRuleRemoveByMsg\ti\n"), the regex matches the tab, but strchr()
returns NULL because no space character exists. The subsequent + 1
produces a pointer value of 0x1 -- undefined behavior.

This is a single systemic bug with 66 manifestation sites.

Other issue (unhandled exception):

Root Cause
----------

The bison-generated parser and flex-generated scanner can throw C++
exceptions in various error conditions (memory allocation failure,
invalid token processing, etc.). The Driver::parse() function calls
parser.parse() without exception handling. An uncaught exception
reaches std::terminate() and aborts the process.

Additionally, if an exception is thrown, scan_end() is never called,
leaking the scanner buffer.

other notes

The bug can only be exploited if the WAF admin puts the pattern above into the config and restarts the engine. This cannot be used remotely or at runtime.

Copy link
Copy Markdown
Contributor

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 potential nullptr dereference / UB in the seclang flex scanner when directives use a tab (\t) as the separator between directive name and argument.

Changes:

  • Added a find_separator() helper to locate either space or tab separators in matched scanner text.
  • Replaced many strchr(yytext, ' ') + 1 call sites with find_separator(yytext) across scanner rules.
  • Updated generated parser/scanner artifacts (seclang-scanner.cc, seclang-parser.hh) to reflect the scanner source changes.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/parser/seclang-scanner.ll Introduces find_separator() and switches many rules to use it (but a few strchr uses remain).
src/parser/seclang-scanner.cc Regenerated flex output incorporating the updated rules/helper.
src/parser/seclang-parser.hh Small generated-header metadata/guard updates.

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

@sonarqubecloud
Copy link
Copy Markdown

@airween airween added the 3.x Related to ModSecurity version 3.x label Apr 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.x Related to ModSecurity version 3.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants