Skip to content

IIS: preserve duplicate request/response headers with same name - #3690

Open
A13501350 wants to merge 1 commit into
owasp-modsecurity:v2/masterfrom
A13501350:fix-2991-iis-duplicate-headers
Open

A13501350 wants to merge 1 commit into
owasp-modsecurity:v2/masterfrom
A13501350:fix-2991-iis-duplicate-headers

Conversation

@A13501350

Copy link
Copy Markdown

Fixes #2991

Problem

On IIS, when multiple response headers share the same name (e.g. Set-Cookie), ModSecurity only records the last one. Consequently, rules using &RESPONSE_HEADERS:/Set-Cookie/ (count) or RESPONSE_HEADERS:Set-Cookie never match earlier values, and audit log part F collapses duplicates to a single entry. The same defect exists for request-side unknown headers.

Root cause

In iis/mymodule.cpp, both OnSendResponse and OnBeginRequest copy unknown headers into the corresponding APR table with apr_table_setn(), which replaces any existing entry with the same key. HTTP.SYS already exposes every duplicate as a separate HTTP_UNKNOWN_HEADER entry in pUnknownHeaders[], but each loop iteration overwrote the previous one, so only the last value survived.

Fix

Switch both unknown-header loops from apr_table_setn() to apr_table_addn(), preserving all values of identically-named headers. ModSecurity's collection generators already iterate over every table entry, so duplicate headers become fully visible to rules and the audit log.

Fixes owasp-modsecurity#2991

Unknown headers were copied into headers_in/headers_out with
apr_table_setn(), so each duplicate overwrote the previous entry and
only the last value survived. HTTP.SYS already exposes every duplicate
as a separate HTTP_UNKNOWN_HEADER entry, so switch both loops to
apr_table_addn() to keep all values. This also makes duplicate
Set-Cookie values reachable: HTTP.SYS keeps one value in the known
header slot and pushes the rest into the unknown header list, where
they were previously lost (and also clobbered the known slot value,
since known headers are transferred first).
@coderabbitai

coderabbitai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 84f0a3ed-02b5-4365-87d6-cefe014fbec9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sonarqubecloud

Copy link
Copy Markdown

@airween airween added the 2.x Related to ModSecurity version 2.x label Oct 15, 2026
@airween

airween commented Oct 15, 2026

Copy link
Copy Markdown
Member

Hi @A13501350,

thanks for this patch - if I'm right, this fixes a bug. Could you add a test case which shows the issue and the correct behavior? The test could be here, with a new target, for eg. 10-response-headers.t.

@A13501350

A13501350 commented Oct 15, 2026 •

Copy link
Copy Markdown
Author

Happy to add tests. However, tests/regression/target looks like the engine + Apache test suite, while this fix is IIS-connector-specific only. I think the ideal approach would be to carve out connector-specific tests first (as done in #3613, where the IIS tests live under iis/tests/) and add them there.

This branch has not been deployed

No deployments
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