fix: record real response status for IIS audit log F part

[A13501350]

Summary

Fixes the IIS connector so the audit log's F part records the real HTTP response status instead of a bogus 500 Internal Server Error.

Problem

In the IIS connector, r->status was never populated from the actual HTTP response. request_rec is apr_pcalloc'd, so r->status stayed 0. When the logging hook builds the F part it calls ap_get_status_line(r->status); for r->status == 0 the standalone ap_index_of_response() maps any value < 100 to LEVEL_500, so every transaction logged HTTP/1.1 500 Internal Server Error regardless of the true response code.

--F--
HTTP/1.1 500 Internal Server Error   <- wrong, e.g. actual response is 200 OK

There was a second, related source of bogus 500 records: the IIS default-document rewrite (e.g. GET / internally redirected to /iisstart.htm) supersedes the original request before it ever reaches OnSendResponse, so its r->status stays 0 and hook_log_transaction wrote a phantom 500 record for it.

Fix

1. In CMyHttpModule::OnSendResponse (iis/mymodule.cpp), transfer the raw response status into r->status (and build r->status_line from the reason phrase) before the rest of the response handling:

if(pRawHttpResponse->StatusCode > 0)
{
    r->status = pRawHttpResponse->StatusCode;

    if(pRawHttpResponse->pReason != NULL && pRawHttpResponse->ReasonLength > 0)
    {
        r->status_line = apr_psprintf(r->pool, "%d %s", r->status,
            ZeroTerminate(pRawHttpResponse->pReason, pRawHttpResponse->ReasonLength, r->pool));
    }
}

2. In hook_log_transaction (apache2/mod_security2.c, under VERSION_IIS), skip audit logging for transactions that never received a response status and were not intercepted. In the IIS connector r->status is only ever set by OnSendResponse, so r->status == 0 && !msr->was_intercepted uniquely identifies requests superseded by an internal redirect:

#if defined(VERSION_IIS)
    if (r->status == 0 && msr->was_intercepted == 0) {
        return DECLINED;
    }
#endif

A blocked request always reaches OnSendResponse with r->status set (e.g. 403), so intercepted transactions are unaffected.

Verification

Verified against a local IIS default site with OWASP CRS (and in the module's Windows CI, which enables the audit log and asserts the F-part status lines):

• Normal GET / now logs HTTP/1.1 200 OK (single record; the phantom 500 records for the internal redirect are gone)
• Blocked request (SQLi/XSS) logs HTTP/1.1 403 ModSecurity Action

Closes #3612

[coderabbitai]

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: Pro Plus

Run ID: ae64ab1b-d54d-44e9-92e3-8fc249c58da3

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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[airween]

Hi @A13501350,

thanks for the fix (and the report too) - could you add some tests to the CI to check this expected behavior? (Not just for this case, but I assume this is important in the future, so if anyone makes some changes, we should be sure the behavior is the same...)

[A13501350]

I can add tests for this. That said, the current workflow is mainly set up to verify that the build compiles and runs. It would be cleaner to test in a dedicated action. Regarding the SonarCloud Quality Gate failure, I'd prefer to keep the original code structure, so I am unable to refactor those lines to meet the maintainability rating.

[airween]

I can add tests for this. That said, the current workflow is mainly set up to verify that the build compiles and runs. It would be cleaner to test in a dedicated action. Regarding the SonarCloud Quality Gate failure, I'd prefer to keep the original code structure, so I am unable to refactor those lines to meet the maintainability rating.

Feel free to add a test case, based on Linux tests. (But the regression tests there use Apache, so probably that won't be good to you. This part probably more useful for first time.)

[A13501350]

web.config exists solely for the CI test environment and never serves real traffic, so I'd consider this ignorable.

[sonarqubecloud]

Quality Gate failed

Failed conditions
B Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

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

[A13501350]

For this specific case I've already added a test. Regarding a full regression suite, since IIS/HTTP.SYS behaves differently from Apache like you said, running the complete CRS suite is quite painful. I only cherry-pick a few tests in CI.

[A13501350]

Like I said before, legacy code is usually not convenient to refactor.