Do not assume ModSecurityIntervention argument to transaction::intervention has been initialized/cleaned #3212
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ention has been initialized/cleaned
- Keep m_it->disruptive value and use it as return value to guarantee
that the value is correct.
- If m_it->disruptive is false and the 'it' argument has not been
initialized/cleaned, the function may incorrectly return a non-zero
value.
- When a disruptive intervention is being reported by the function,
defensively initialize log & url to NULL if there's no such data to
provide to the caller.
- If the caller has not initialized/cleaned those fields in the 'it'
argument, after returning from transaction::intervention, the user
can safely read the log & url fields and in all scenarios they'll
have valid values.
- Initialize `log` temporary value on construction instead of doing default initialization and then calling `append`. - Leverage `std::string_view` to replace `const std::string&` parameters in `utils::string::replaceAll` to avoid creating a `std::string` object (and associated allocation and copy) for the string literal`%d`
…on Unix builds - This configuration flag was introduced in commit d47185d in the context of PR owasp-modsecurity#3207. - Moved to the configure step's 'run' command in order to be shared across configurations. - For the sake of reference, matrix.platform.configure should be used for configuration flags that are needed for a specific platform/architecture (which was the reason it was introduced in commit d9255d8, PR owasp-modsecurity#3144).
|
|
Thanks @eduar-hte! |
airween
added a commit
to airween/ModSecurity
that referenced
this pull request
Aug 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
what
Minor changes to transaction::intervention to guarantee to caller that all members of the
ModSecurityInterventionhave been initialized if an intervention should be made and that the return value of the function is consistent with the state of the transaction.why
The current version of Transaction::intervention assumes (and implicitly requires) that the
ModSecurityInterventionargument has been initialized/cleaned before the call.This is not necessarily expected because the parameter is used as an output parameter and it's not clear that any input is provided to the function.
Additionally, in order to make the API more robust, when the function returns that an intervention should be made, initialize all members in the structure to guarantee to the caller that all of them can be safely accessed and interpreted.
changes
m_it->disruptivevalue and use it as return value to guarantee that the value is correct.m_it->disruptiveisfalseand theitargument has not been initialized/cleaned, the function may incorrectly return a non-zero value.log&urlfields toNULLif there's no such data to provide to the caller.itargument, after returning fromtransaction::intervention, the user can safely read thelog&urlfields (and call cleanup functions such asintervention::freeormsc_intervention_cleanup-introduced in PR Add cleanup methods to complete C based ABI #3209).Transaction::intervention&msc_intervention.