Skip to content

Conversation

@vkrivopalov
Copy link

Description

Previously, calls to msre_generate_target_string() from inside
update_rule_target_ex() would accumulate memory allocations from ruleset
memory pool that is never released. For reasonably large exclusion lists
memory consumption grows exponentially for no good reason.

This fix introduces the use of local memory pool for all intermediate
operations that is destroyed upon completion. This ensures that all
memory reallocations used for building strings are properly released.

Testing

Running Nginx with ModSecurity no user-defined exclusions:

# ps aux
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root     32371  0.0  0.2 220044 18408 ?        Ss   09:53   0:00 nginx: master process nginx
www-data 32372  0.6  0.6 542756 51520 ?        Sl   09:53   0:00 nginx: worker process

Running Nginx with ModSecurity in prevention mode, 40 user-defined exclusions of the following type:

SecRuleUpdateTargetById 10001-999999 "!ARGS:'/^param/',!ARGS_GET:'/^param/',!ARGS_POST:'/^param/'"

before the fix:

# ps aux
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root     32680  0.3 15.7 1477184 1264004 ?     Ss   09:56   0:00 nginx: master process nginx
www-data 32681  0.3 16.1 1799896 1297096 ?     Sl   09:56   0:00 nginx: worker process

Running Nginx with ModSecurity, 40 user-defined exclusions, after the fix:

# ps aux
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root      1046  0.0  2.6 425760 212676 ?       Ss   09:58   0:00 nginx: master process nginx
www-data  1047  2.0  3.0 748472 245736 ?       Sl   09:58   0:00 nginx: worker process

Checked that configured exclusions apply, i.e., if an excluded parameter contains a malicious string the request is not blocked.

… footprint

Previously, calls to msre_generate_target_string() from inside
update_rule_target_ex() would accumulate memory allocations from ruleset
memory pool that is never released. For reasonably large exclusion lists
memory consumption grows exponentially for no good reason.

This fix introduces the use of local memory pool for all intermediate
operations that is destroyed upon completion. This ensures that all
memory reallocations used for building strings are properly released.

Signed-off-by: Vladimir Krivopalov <vladimir.krivopalov@gmail.com>
@zimmerle zimmerle requested review from martinhsv and zimmerle October 3, 2019 11:52
@zimmerle zimmerle self-assigned this Oct 3, 2019
@zimmerle zimmerle added the 3.x Related to ModSecurity version 3.x label Oct 3, 2019
@zimmerle zimmerle added this to the v3.1.0 milestone Oct 3, 2019
@zimmerle zimmerle added 2.x Related to ModSecurity version 2.x and removed 3.x Related to ModSecurity version 3.x labels Oct 3, 2019
Copy link

@marcstern marcstern left a comment

Choose a reason for hiding this comment

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

Running in prod for many months

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.x Related to ModSecurity version 2.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants