You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2011/07/13 15:28:24 UTC
svn commit: r1146005 - in /tomcat/trunk/java/org/apache/catalina/connector:
LocalStrings.properties Request.java
Author: markt
Date: Wed Jul 13 13:28:24 2011
New Revision: 1146005
URL: http://svn.apache.org/viewvc?rev=1146005&view=rev
Log:
When running under a security manager and using sendfile, validate sendfile attributes to prevent sendfile being used to bypass the security manager.
Part of the fix for CVE-2011-2526
Modified:
tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties
tomcat/trunk/java/org/apache/catalina/connector/Request.java
Modified: tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties?rev=1146005&r1=1146004&r2=1146005&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties Wed Jul 13 13:28:24 2011
@@ -66,6 +66,7 @@ coyoteRequest.noLoginConfig=No authentic
coyoteRequest.authenticate.ise=Cannot call authenticate() after the reponse has been committed
coyoteRequest.uploadLocationInvalid=The temporary upload location [{0}] is not valid
coyoteRequest.sessionEndAccessFail=Exception triggered ending access to session while recycling request
+coyoteRequest.sendfileNotCanonical=Unable to determine canonical name of file [{0}] specified for use with sendfile
requestFacade.nullRequest=The request object has been recycled and is no longer associated with this facade
Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?rev=1146005&r1=1146004&r2=1146005&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Wed Jul 13 13:28:24 2011
@@ -1525,6 +1525,26 @@ public class Request
return;
}
+ // Do the security check before any updates are made
+ if (Globals.IS_SECURITY_ENABLED &&
+ name.equals("org.apache.tomcat.sendfile.filename")) {
+ // Use the canonical file name to avoid any possible symlink and
+ // relative path issues
+ String canonicalPath;
+ try {
+ canonicalPath = new File(value.toString()).getCanonicalPath();
+ } catch (IOException e) {
+ throw new SecurityException(sm.getString(
+ "coyoteRequest.sendfileNotCanonical", value), e);
+ }
+ // Sendfile is performed in Tomcat's security context so need to
+ // check if the web app is permitted to access the file while still
+ // in the web app's security context
+ System.getSecurityManager().checkRead(canonicalPath);
+ // Update the value so the canonical path is used
+ value = canonicalPath;
+ }
+
oldValue = attributes.put(name, value);
if (oldValue != null) {
replaced = true;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1146005 - in /tomcat/trunk/java/org/apache/catalina/connector:
LocalStrings.properties Request.java
Posted by sebb <se...@gmail.com>.
On 22 August 2011 16:03, Konstantin Kolinko <kn...@gmail.com> wrote:
> 2011/8/22 sebb <se...@gmail.com>:
>> On 13 July 2011 14:28, <ma...@apache.org> wrote:
>>> Author: markt
>>> Date: Wed Jul 13 13:28:24 2011
>>> New Revision: 1146005
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1146005&view=rev
>>> Log:
>>> When running under a security manager and using sendfile, validate sendfile attributes to prevent sendfile being used to bypass the security manager.
>>> Part of the fix for CVE-2011-2526
>>>
>>> Modified:
>>> tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties
>>> tomcat/trunk/java/org/apache/catalina/connector/Request.java
>>>
>
>>> --- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
>>> +++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Wed Jul 13 13:28:24 2011
>>> @@ -1525,6 +1525,26 @@ public class Request
>>> return;
>>> }
>>>
>>> + // Do the security check before any updates are made
>>> + if (Globals.IS_SECURITY_ENABLED &&
>>> + name.equals("org.apache.tomcat.sendfile.filename")) {
>>
>> IMO this "magic string" should be a constant - as is done earlier in the file:
>>
>> ... name.equals(Globals.DISPATCHER_REQUEST_PATH_ATTR) ...
>>
>
> You are right. Actually there are three magic strings used by sendfile
> (filename + range bounds).
>
> (It could not be done in r1146005 in order to reduce noise in a security patch).
I see.
In which case there are several other related magic strings in
DefaultServlet and Http11AprProcessor and Http11NioProcessor.
Probably elsewhere too; these are just the files that use
"org.apache.tomcat.sendfile.filename".
> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1146005 - in /tomcat/trunk/java/org/apache/catalina/connector:
LocalStrings.properties Request.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/8/22 sebb <se...@gmail.com>:
> On 13 July 2011 14:28, <ma...@apache.org> wrote:
>> Author: markt
>> Date: Wed Jul 13 13:28:24 2011
>> New Revision: 1146005
>>
>> URL: http://svn.apache.org/viewvc?rev=1146005&view=rev
>> Log:
>> When running under a security manager and using sendfile, validate sendfile attributes to prevent sendfile being used to bypass the security manager.
>> Part of the fix for CVE-2011-2526
>>
>> Modified:
>> tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties
>> tomcat/trunk/java/org/apache/catalina/connector/Request.java
>>
>> --- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
>> +++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Wed Jul 13 13:28:24 2011
>> @@ -1525,6 +1525,26 @@ public class Request
>> return;
>> }
>>
>> + // Do the security check before any updates are made
>> + if (Globals.IS_SECURITY_ENABLED &&
>> + name.equals("org.apache.tomcat.sendfile.filename")) {
>
> IMO this "magic string" should be a constant - as is done earlier in the file:
>
> ... name.equals(Globals.DISPATCHER_REQUEST_PATH_ATTR) ...
>
You are right. Actually there are three magic strings used by sendfile
(filename + range bounds).
(It could not be done in r1146005 in order to reduce noise in a security patch).
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1146005 - in /tomcat/trunk/java/org/apache/catalina/connector:
LocalStrings.properties Request.java
Posted by sebb <se...@gmail.com>.
On 13 July 2011 14:28, <ma...@apache.org> wrote:
> Author: markt
> Date: Wed Jul 13 13:28:24 2011
> New Revision: 1146005
>
> URL: http://svn.apache.org/viewvc?rev=1146005&view=rev
> Log:
> When running under a security manager and using sendfile, validate sendfile attributes to prevent sendfile being used to bypass the security manager.
> Part of the fix for CVE-2011-2526
>
> Modified:
> tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties
> tomcat/trunk/java/org/apache/catalina/connector/Request.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties?rev=1146005&r1=1146004&r2=1146005&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties (original)
> +++ tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties Wed Jul 13 13:28:24 2011
> @@ -66,6 +66,7 @@ coyoteRequest.noLoginConfig=No authentic
> coyoteRequest.authenticate.ise=Cannot call authenticate() after the reponse has been committed
> coyoteRequest.uploadLocationInvalid=The temporary upload location [{0}] is not valid
> coyoteRequest.sessionEndAccessFail=Exception triggered ending access to session while recycling request
> +coyoteRequest.sendfileNotCanonical=Unable to determine canonical name of file [{0}] specified for use with sendfile
>
> requestFacade.nullRequest=The request object has been recycled and is no longer associated with this facade
>
>
> Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?rev=1146005&r1=1146004&r2=1146005&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Wed Jul 13 13:28:24 2011
> @@ -1525,6 +1525,26 @@ public class Request
> return;
> }
>
> + // Do the security check before any updates are made
> + if (Globals.IS_SECURITY_ENABLED &&
> + name.equals("org.apache.tomcat.sendfile.filename")) {
IMO this "magic string" should be a constant - as is done earlier in the file:
... name.equals(Globals.DISPATCHER_REQUEST_PATH_ATTR) ...
> + // Use the canonical file name to avoid any possible symlink and
> + // relative path issues
> + String canonicalPath;
> + try {
> + canonicalPath = new File(value.toString()).getCanonicalPath();
> + } catch (IOException e) {
> + throw new SecurityException(sm.getString(
> + "coyoteRequest.sendfileNotCanonical", value), e);
> + }
> + // Sendfile is performed in Tomcat's security context so need to
> + // check if the web app is permitted to access the file while still
> + // in the web app's security context
> + System.getSecurityManager().checkRead(canonicalPath);
> + // Update the value so the canonical path is used
> + value = canonicalPath;
> + }
> +
> oldValue = attributes.put(name, value);
> if (oldValue != null) {
> replaced = true;
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Tagging 7.0.19
Posted by Rainer Jung <ra...@kippdata.de>.
On 13.07.2011 15:32, Mark Thomas wrote:
> On 13/07/2011 14:28, markt@apache.org wrote:
>> Author: markt
>> Date: Wed Jul 13 13:28:24 2011
>> New Revision: 1146005
>
> With this commit I am going to start the process of running the unit and
> TCK tests prior to tagging 7.0.19. Assuming everything passes, expect
> the tag in the next 10 hours or so.
+1
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Tagging 7.0.19
Posted by Mark Thomas <ma...@apache.org>.
On 13/07/2011 14:28, markt@apache.org wrote:
> Author: markt
> Date: Wed Jul 13 13:28:24 2011
> New Revision: 1146005
With this commit I am going to start the process of running the unit and
TCK tests prior to tagging 7.0.19. Assuming everything passes, expect
the tag in the next 10 hours or so.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org