Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: handle error with SecConnReadStateLimit configuration #2834

Merged
merged 1 commit into from
Nov 20, 2022

Conversation

martinhsv
Copy link
Contributor

Fix NULL-memory use case if an admin has not correctly included both an operator and a parameter (with a space-character separator).

@martinhsv martinhsv merged commit 31c3b4e into owasp-modsecurity:v2/master Nov 20, 2022
@martinhsv
Copy link
Contributor Author

Addresses the main issue raised in #2815

@marcstern
Copy link
Contributor

The third parameter is optional, thus the test should be:
if (!param && *p2)
This will accept

  • SecConnReadStateLimit 50 "@ipMatch 127.0.0.1" ...
  • SecConnReadStateLimit 50
    But will refuse
  • SecConnReadStateLimit 50 "127.0.0.1" ...

@martinhsv
Copy link
Contributor Author

Hi @marcstern ,

Perhaps I'm not understanding what you mean.

But within parser_conn_limits_operator() we don't need to check whether p2 is NULL or not because parser_conn_limits_operator() will not be called at all if p2 is NULL.

And I did do some testing prior to the merge and it seemed to work per your summary. What use case do you believe is not working correctly?

@marcstern
Copy link
Contributor

Hi @martinhsv,
Indeed, there's a test before the call.
Sorry for the mistake.

@martinhsv
Copy link
Contributor Author

No worries.

@martinhsv martinhsv added the 2.x Related to ModSecurity version 2.x label Nov 22, 2022
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.

2 participants