Skip to content

Check for null pointer dereference (almost) everywhere #3120

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

Merged
merged 20 commits into from
May 6, 2024
Merged

Check for null pointer dereference (almost) everywhere #3120

merged 20 commits into from
May 6, 2024

Conversation

marcstern
Copy link

Added a design doc explaining the approach

@marcstern marcstern requested a review from airween April 4, 2024 13:48
@airween airween self-assigned this Apr 8, 2024
Copy link
Contributor

@fzipi fzipi left a comment

Choose a reason for hiding this comment

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

The use of assertions will terminate the process, right? Is that what you want?

@airween
Copy link
Member

airween commented Apr 15, 2024

The use of assertions will terminate the process, right? Is that what you want?

You can control the behavior with a macro. See the identical solution in case of libmodsecurity3. Here @gberkes introduced a new CFLAG, which can be controlled with a configure option. It's disabled by default, so the process won't terminate - but we can use that in our test environment.

@marcstern
Copy link
Author

The use of assertions will terminate the process, right? Is that what you want?

Yes, as it's an impossible path, unless the implementation is bogus

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
22.0% Duplication on New Code (required ≤ 3%)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@airween airween added the 2.x Related to ModSecurity version 2.x label May 3, 2024
@airween
Copy link
Member

airween commented May 6, 2024

Looks good to me. I think the SonarCloud reports are false positives, all mentioned duplicate code had added years ago. Merging now - thanks, Marc.

@airween airween merged commit d9016e2 into owasp-modsecurity:v2/master May 6, 2024
40 of 41 checks passed
@marcstern marcstern deleted the v2/mst/nullcheck2 branch May 28, 2024 11:37
@marcstern marcstern restored the v2/mst/nullcheck2 branch June 7, 2024 12:10
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