IIS: Host header fallback is dead code (r->hostname == NULL can never be true)

Open Beginner friendly
#3,621 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
82/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp
Domain
backend, security

Research direction

Start in iis/mymodule.cpp around lines 840-853 and review ConvertUTF16ToUTF8 at lines 180-184, 199-202, and 226-229. Update the fallback condition to handle an empty hostname, then verify that an HTTP/1.1 request with a Host header populates r->hostname and r->parsed_uri.hostname when CookedUrl.pHost is empty.

Written by the indexing model from the issue text.

Description

2.x Platform - IIS

Summary

In iis/mymodule.cpp, the Host header fallback when the request URI contains no host is dead code, because r->hostname == NULL can never be true.

r->hostname is assigned from ConvertUTF16ToUTF8(req->CookedUrl.pHost, ...) (line 840). That helper never returns NULL: on NULL/empty input, zero converted bytes, or conversion error it returns the string literal "" (see mymodule.cpp:180-184, :199-202, :226-229); on success it returns a pool-allocated buffer. So after line 840 r->hostname is always non-NULL (an empty string when the URI has no host).

As a result:

  • mymodule.cpp:843 if(r->hostname == NULL) is always false → the fallback to req->Headers.KnownHeaders[HttpHeaderHost] never runs.
  • mymodule.cpp:853 if(r->hostname != NULL) is always true.

Impact

For ordinary HTTP/1.1 requests (GET /path with a Host: header, where the request line carries no host), CookedUrl.pHost is empty, so r->hostname becomes "" instead of the value from the Host header. The intended fallback is silently skipped, leaving r->hostname / r->parsed_uri.hostname empty. Hostname-dependent rules and logging may see an empty host.

Suggested fix

Check for an empty string as well as NULL (keeps the helper's contract intact for the other callers path_info/args):

    r->hostname = ConvertUTF16ToUTF8(req->CookedUrl.pHost, req->CookedUrl.HostLength / sizeof(WCHAR), r->pool);

    if(r->hostname == NULL || r->hostname[0] == '\0')
    {
        if(req->Headers.KnownHeaders[HttpHeaderHost].pRawValue != NULL)
            r->hostname = ZeroTerminate(req->Headers.KnownHeaders[HttpHeaderHost].pRawValue,
                                        req->Headers.KnownHeaders[HttpHeaderHost].RawValueLength, r->pool);
    }
Dominant language
C++
Stars
9.8k
Forks
1.8k
Avg merge
2h 46m
Merged PRs (30d)
1

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from owasp-modsecurity/ModSecurity

All issues in owasp-modsecurity/ModSecurity

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.