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.

wegge wrote:

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().