Extra modification#3520
Conversation
|
There was a problem hiding this comment.
Pull request overview
Adds defensive handling and extra Apache IfDefine tokens for ModSecurity, likely to prevent crashes and advertise feature availability at config time.
Changes:
- Add a NULL-guard in
construct_single_var()aroundmsc_rule_mptmp. - Add additional
MODSEC_*entries toap_server_config_defines, including a PCRE2 indicator.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apache2/msc_util.c | Adds an early return when the per-rule temp pool (msc_rule_mptmp) is missing during variable construction. |
| apache2/mod_security2.c | Pushes additional MODSEC_* defines into Apache’s ap_server_config_defines for conditional configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1664,7 +1664,11 @@ static void register_hooks(apr_pool_t *mp) { | |||
|
|
|||
| /* Add the MODSEC_a.b define */ | |||
There was a problem hiding this comment.
The comment says "Add the MODSEC_a.b define" but this block now adds multiple defines (including feature/compile-mode defines). Update the comment (or split it) so it accurately describes what is being added.
| /* Add the MODSEC_a.b define */ | |
| /* Add version, feature, and compile-mode ModSecurity defines. */ |
| msre_var *vx = NULL; | ||
| char *my_error_msg = NULL; | ||
|
|
||
| if (msr->msc_rule_mptmp == NULL) return NULL; |
There was a problem hiding this comment.
construct_single_var now returns NULL whenever msr->msc_rule_mptmp is NULL. msr->msc_rule_mptmp is initialized to NULL at transaction creation (apache2/mod_security2.c:582) and is only created during rule processing (apache2/re.c:3362-3368), so requests that reach logging without any rule execution will stop producing log variables (instead of being computable from msr->mp). Consider lazily creating msc_rule_mptmp here (e.g., apr_pool_create with parent msr->mp) or otherwise providing a safe fallback pool rather than returning NULL.
This is a Marc Stern modification, I don't have much more insight on the code he made. To be reviewed with caution and check if this is still relevant.
I'm not sure from which commit Marc made those modifications.