Skip to content

fix: remove unwanted '\0' string terminator from argument's value #3411

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 1 commit into from
Jul 4, 2025

Conversation

airween
Copy link
Member

@airween airween commented Jul 2, 2025

what

This PR removes the unwanted \0 character from the end of parsed argument value, if the SecParseXmlIntoArgs feature is enabled and if the node is empty.

why

Now if the node is empty the engine adds an extra byte to the argument's value, which is the string terminator. That character will be part of the value, so if the node is <foo></foo>, the value will be \0 with length 1. Then the parsed argument will be xml.foo=\x00 - which can cause false positives.

@airween
Copy link
Member Author

airween commented Jul 2, 2025

Cc: @RedXanadu

@airween airween added the 2.x Related to ModSecurity version 2.x label Jul 2, 2025
@airween airween requested a review from theseion July 2, 2025 19:53
Copy link

sonarqubecloud bot commented Jul 2, 2025

Copy link

@RedXanadu RedXanadu left a comment

Choose a reason for hiding this comment

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

I've tested this over the past few days and can confirm this PR resolves the issue where stray null characters were appearing in ARGS and causing lots of false positives.

No false positives and no unexpected null chars after applying this change.

@airween airween merged commit 981d225 into owasp-modsecurity:v2/master Jul 4, 2025
82 checks passed
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