Skip to content

fix: probably UB (left shift of neg. val) in ip_tree#3541

Open
airween wants to merge 1 commit intoowasp-modsecurity:v3/masterfrom
airween:v3/iptree
Open

fix: probably UB (left shift of neg. val) in ip_tree#3541
airween wants to merge 1 commit intoowasp-modsecurity:v3/masterfrom
airween:v3/iptree

Conversation

@airween
Copy link
Copy Markdown
Member

@airween airween commented Apr 12, 2026

what

This PR fixes a possible undefined behavior (UB) in IP TREE during the execution of @ipMatch.

why

There is a bug report, received in email from @fumfel and his team. Also they provided this fix.

references

The original report:

Root Cause
----------

The SHIFT_LEFT_MASK(x) macro computes (-1) << (x) to create a bitmask
with x zero bits on the right. On two's complement architectures with
arithmetic shifts, this produces the expected result. However, it is
formally undefined behavior per the C and C++ standards. Aggressive
compiler optimizations (such as -fstrict-overflow) could produce
unexpected results.

The bug triggers when processing IP addresses for @ipMatch with certain
netmask values. Input "\0::1" (null byte + IPv6 loopback notation)
reaches CPTFindElement where ip_bitmask % 8 == 0, producing
(-1) << 8.

other notes

The bug can only be exploited if the source code was built with different flags, like -fsanitize=address,undefined -fno-sanitize-recover=all and the admin puts a \0 byte into operator's argument on some way.

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes a case of undefined behavior in the IP tree bitmask logic used by the @ipMatch operator by avoiding left-shifting a negative signed value.

Changes:

  • Update SHIFT_LEFT_MASK(x) to build the mask via an unsigned all-ones value (~0U) before shifting, eliminating UB from (-1) << x.
  • Keep the rest of the IP tree matching logic unchanged while making sanitizer/optimization behavior predictable.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@airween airween added the 3.x Related to ModSecurity version 3.x label Apr 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.x Related to ModSecurity version 3.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants