Hi 

I'm using Tomcat 6.0.35 on Windows 7 and RHEL 6.x.   I think I've stumbled upon 
a bug in org.apache.catalina.valves.RemoteIpValve.   

The issue is related to using commas in any regular expressions used with the 
"internalProxies" or "trustedProxies" attributes.  

*** Steps to reproduce ***

Add the following XML to $CATALINA_HOME/conf/context.xml  under the Context 
element: 

    <Valve className="org.apache.catalina.valves.RemoteIpValve"
        internalProxies="10\.\d{1,3}\.\d{1,3}\.\d{1,3}"
        protocolHeader="Example-Header"
        protocolHeaderHttpsValue="example-value" />

The XML above is based on the regex given on the pages 
http://tomcat.apache.org/tomcat-6.0-doc/config/valve.html and 
http://tomcat.apache.org/tomcat-6.0-doc/api/org/apache/catalina/valves/RemoteIpValve.html:
 

    10\.\d{1,3}\.\d{1,3}\.\d{1,3}, 192\.168\.\d{1,3}\.\d{1,3}, 
169\.254\.\d{1,3}\.\d{1,3}, 127\.\d{1,3}\.\d{1,3}\.\d{1,3}

*** Symptoms of the bug ***

RemoteIpValve incorrectly splits the ="10\.\d{1,3}\.\d{1,3}\.\d{1,3}" string 
into the two regular expressions "10\.\d{1" and "3}\.\d{1,3}\.\d{1,3}" 
(RemoteIpValve.java line 396).  In then attempts to parse the first regular 
expression which throws a PatternSyntaxException (line 382).  The class catches 
this and throws a IllegalArgumentException (line 384).  

Because an exemption is thrown, the "internalProxies" will keep its default 
value (set on line 445) instead of being cleared.  This is confusing for 
someone trying to diagnose why the value they set "internalProxies" to is not 
working.  It would be better if the class cleared the value of 
"internalProxies" in this case.  

*** Suggested fix ***

1) Instead of throwing the IllegalArgumentException, the class could log a 
debug message against the class's logger with information on the string that 
could not be parse as a regular expression.  This would a) help people with 
identifying and diagnosing the issue and b) stop RemoteIpValve from falling 
back to its default list of "internalProxies".  

2) When internalProxies is initialized (line 445), use 
commaDelimitedListToPatternArray to parse the default regular expressions from 
a single string instead of parsing the four regular expressions separately.  
This change would instantly flag up any issues (e.g. a comma) in the default 
regular expressions.  

3) Update the RemoteIpValve documentation 
http://tomcat.apache.org/tomcat-6.0-doc/config/valve.html and 
http://tomcat.apache.org/tomcat-6.0-doc/api/org/apache/catalina/valves/RemoteIpValve.html
 to remove the commas from the documentation's example regular expressions.  
For example, this regular expression 

    10\.\d{1,3}\.\d{1,3}\.\d{1,3}, 192\.168\.\d{1,3}\.\d{1,3}, 
169\.254\.\d{1,3}\.\d{1,3}, 127\.\d{1,3}\.\d{1,3}\.\d{1,3}

could be replaced with 

    10\.\d+\.\d+\.\d+, 192\.168\.\d+\.\d+, 169\.254\.\d+\.\d+, 
127\.\d+\.\d+\.\d+

4) Update the class's documentation to make it clear that commas should not be 
used in the regular expressions, other than to separate multiple regular 
expressions.  

*** Other things that might be affected by the same bug ***

It's worth cheching whether RemoteIpValve  and RemoveIpFilter in Tomcat 7.x are 
affected by the same bug.  


Kind regards 
Simon Dean
  

-----------------------------------------------------------------------------------------------------------------------------------------
The information contained in this message may be CONFIDENTIAL and is intended 
for the addressee only. Any unauthorised use, dissemination of the information, 
or copying of this message is prohibited. If you are not the addressee, please 
notify the sender immediately by return e-mail and delete this message. 
Although this e-mail and any attachments are believed to be free of any virus, 
or other defect which might affect any computer or system into which they are 
received and opened, it is the responsibility of the recipient to ensure that 
they are virus free and no responsibility is accepted by Moneysupermarket.com 
Financial Group Limited for any loss or damage from receipt or use thereof. 
The views expressed are of the individual, and do not necessarily reflect the 
views of Moneysupermarket.com Financial Group Limited.
Moneysupermarket.com Limited is an appointed representative of 
Moneysupermarket.com Financial Group Limited, which is authorised and regulated 
by the Financial Services Authority (FSA FRN 303190). 
Moneysupermarket.com Financial Group Limited, registered in England No. 
3157344. 
Registered Office: Moneysupermarket House, St. David’s Park, Ewloe, CH5 3UZ. 
Telephone 01244 665700.


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to