Last modified: 2010-05-15 16:03:16 UTC
According to RFC 2616 section 4.2, headers like X-Forwarded-for are case insensitive: Each header field consists of a name followed by a colon (":") and the field value. Field names are case-insensitive. However, ProxyTools.php looks for a specific case, that is "X-Forwarded-For", which means that edits coming through a proxy not using this specific case of the header will be registered as coming from the proxy itself, no matter the content of $wgSquidServers.
The important part of our checks is to ensure that a malicious client that sends a header like "X_Forwarded_For" can't override the *real* "X-Forwarded-For" header. This is a problem with how HTTP headers from the server get stuck into the $_SERVER array (which follows the CGI spec, folding '-' and '_'.) In theory, any intermediate proxy server should merge all legitimate case variations into a single one, so it should indeed be safe to do a case-insensitive check on the final apache_request_headers() array. WebRequest::getHeader() appears to be doing this check correctly. If WebRequest is safe to use at ProxyTools time, the sensible thing would be to go ahead and just change the ProxyTools bits to use that function. If not, the two should be merged to a sane shared backend function.
Using WebRequest::getHeader would be unsafe in this case, as it gives no indication whether it's the case-folded header it's returning. Thus a header: Client-ip: 127.0.0.1 X_Forwarded_For: spoof Could in theory return the spoofed value. The obvious choice is to handle this in a single place, where getHeader() surely is the logical choice. For now I've added the appropriate strtoupper() calls in wfGetForwardedFor() and wfGetAgent().
