Unplanned
Last Updated: 11 Feb 2021 16:48 by Eric
Ray
Created on: 04 Feb 2021 06:17
Type: Bug Report
2
Bug in HostnameIs when host has port (eg test:81 or CONNECT test:443)

The bug I'm reporting is sometimes the Session.HostnameIs() will return true even if the supplied hostname does not match Session.hostname and a port was passed by the client in the Host header.

HostnameIs function is documented as "This method compares the supplied hostname to the hostname of the request, returning true if a case-insensitive match is found."

What I think is happening is that rather than use Session.hostname for comparison Fiddler instead uses the Session.host (ie what was passed by the client in the Host header) and if a port is present maybe it incorrectly extracts out the hostname. Here is an example that shows the bug and why I think that.

In OnBeforeRequest add this code, which should only show an alert box if the hostname is test:

		if(oSession.HostnameIs("test")) {
			FiddlerObject.alert(oSession.hostname);
		}

Now in a browser try going to http://t:81/ and you will see it shows the alert box, in other words a match. Why? Well, I will guess based on my testing that your code in HostnameIs gets the index of the colon in the host t:81, which is 1, and then compares only that number of characters. So it's doing whatever is the javascript equivalent of !strnicmp("t", "test", 1).

This manifests itself through CONNECT as well, and probably more likely, since the standard ports are used in the Host header (IE might be an exception to this). For example, let's say you go to https://t/ in Firefox or Chrome and HTTPS decrypt is enabled. The Host passed by the client for the CONNECT is t:443 and so it's the same problem, !strnicmp("t", "test", 1).

This is not a theoretical issue for me, I was testing something earlier today where I had to treat a hostname that ended in .co different from the same hostname that ended in .com and it turned out the test I was doing applied to both of them because of this bug.

There may be very good reason to not use Session.hostname for the comparison, I don't know, but the likely extraction from Session.host is not done properly.
2 comments
Eric
Posted on: 11 Feb 2021 16:48

What a delightfully fascinating bug that's over a decade old-- thanks for reporting it!

The problem is that the code looks like this:

 int iColon = oRequest.host.LastIndexOf(':');
 if ((iColon > -1) && (iColon > oRequest.host.LastIndexOf(']')))
 {
     return (0 == String.Compare(oRequest.host, 0, sToMatch, 0, iColon, StringComparison.OrdinalIgnoreCase));
 }

The goal here (vs. using the .hostname property) was to avoid allocations, but it's obviously incorrect to do it this way. We instead should be comparing the entire sTestHost string (e.g. using sToMatch.Length rather than iColon) in the .Compare call.

In terms of workaround, you should get the right behavior from

 

   if (StringExtensions.OICEquals(oSession.hostname, "thingtomatch")) {

      //...

  }

 

Ray
Posted on: 04 Feb 2021 06:54

I tried what I thought would be a good workaround but it's not working:

oSession.hostname.Equals("test2", StringComparison.OrdinalIgnoreCase)

Fiddler throws an error saying too many arguments and will ignore them. It doesn't even do that though, it doesn't appear to work at all. I can see in the Equals overloads tooltip in the Fiddler ScriptEditor that there is a proper one, so I'm not sure what's happening. The 3rd overload is value: String, comparisonType: StringComparison

This also fails:

String.Equals(oSession.hostname, "test2", StringComparison.OrdinalIgnoreCase)

This works:

!String.Compare(oSession.hostname, "test2", StringComparison.OrdinalIgnoreCase)

 

Attached Files: