Bug 50026 - DefaultServlet serves META-INF and WEB-INF from root when remapped on /folder/*
DefaultServlet serves META-INF and WEB-INF from root when remapped on /folder/*
Status: RESOLVED FIXED
Product: Tomcat 6
Classification: Unclassified
Component: Catalina
6.0.29
PC Windows XP
: P3 enhancement (vote)
: default
Assigned To: Tomcat Developers Mailing List
:
: 50153 (view as bug list)
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2010-09-29 11:25 UTC by balusc
Modified: 2014-02-17 13:53 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description balusc 2010-09-29 11:25:46 UTC
The following in web.xml

    <servlet>
        <servlet-name>static</servlet-name>
        <servlet-class>org.apache.catalina.servlets.DefaultServlet</servlet-class>
    </servlet>
    <servlet-mapping>
        <servlet-name>static</servlet-name>
        <url-pattern>/static/*</url-pattern>
    </servlet-mapping>

makes restricted folders accessible by e.g.

http://localhost:8080/context/static/WEB-INF/web.xml
http://localhost:8080/context/static/META-INF/MANIFEST.MF
Comment 1 Pid 2010-09-29 11:57:22 UTC
(In reply to comment #0)
> The following in web.xml
> 
>     <servlet>
>         <servlet-name>static</servlet-name>
>        
> <servlet-class>org.apache.catalina.servlets.DefaultServlet</servlet-class>
>     </servlet>
>     <servlet-mapping>
>         <servlet-name>static</servlet-name>
>         <url-pattern>/static/*</url-pattern>
>     </servlet-mapping>

URL patterns are application-relative.

> makes restricted folders accessible by e.g.
> 
> http://localhost:8080/context/static/WEB-INF/web.xml
> http://localhost:8080/context/static/META-INF/MANIFEST.MF

The directories you refer to aren't in use by the app called 'context', so they're just ordinary directories which happen to duplicate the special name. The actual special directories would be here:

 http://localhost:8080/context/WEB-INF/web.xml 
 http://localhost:8080/context/META-INF/MANIFEST.MF
Comment 2 balusc 2010-09-29 12:08:27 UTC
No, there are no directories like that in `/static` folder. It actually refers to the restricted directories in the context root.
Comment 3 Chuck Caldarale 2010-09-29 12:34:42 UTC
(In reply to comment #2)
> No, there are no directories like that in `/static` folder. It actually refers
> to the restricted directories in the context root.

The standard DefaultServlet uses just HttpServletRequest.getPathInfo(), which is defined to return only extra path information - that which is beyond the mapping that selected the servlet.  So, with the "/static/*" pattern and a URL of /context/static/WEB-INF/web.xml, the DefaultServlet went looking for "/WEB-INF/web.xml" - and found it.

I think what would be best is to change the DefaultServlet so it includes both getServletPath() and getPathInfo() in its result string, rather than just one or the other.  I haven't tried it yet, but I think the following replacement for the getRelativePath() method in DefaultServlet will work:

    protected String getRelativePath(HttpServletRequest request) {
        // Are we being processed by a RequestDispatcher.include()?
        if (request.getAttribute(Globals.INCLUDE_REQUEST_URI_ATTR) != null) {
            String result = (String)request.getAttribute(Globals.INCLUDE_PATH_INFO_ATTR);
            if (result == null) {
                result = (String)request.getAttribute(Globals.INCLUDE_SERVLET_PATH_ATTR);
            }
            if (result == null || result.equals("")) result = "/";
            return result;
        }
        // No, extract the desired path directly from the request.
        String result = request.getPathInfo();
        if (result == null) {
            result = request.getServletPath();
        } else {
            result = request.getServletPath() + result;
        }
        if (result == null || result.equals("")) result = "/";
        return result;
    }

You could also write your own class that extends DefaultServlet, and override the getRelativePath() method with the above.

 - Chuck
Comment 4 Tim Whittington 2010-10-03 04:05:36 UTC
I'm thinking this is a WONTFIX.

The servlet engine protects the WEB-INF and META-INF paths in the web application (which is working fine), not files of that name under arbitrary paths.

What's actually happening here is you're configuring a general purpose file serving servlet to mount up your entire web application under a different path - it's equivalent to configuring Apache to do the same thing. Except that DefaultServlet isn't a general purpose file server - it's designed to be mapped to /, and you can't configure it to do anything but serve files out of the web application directory.

I'm guessing you're trying to work around a problem introduced by mapping another servlet to /*, which is basically trying to work around the way a servlet engine works.
http://stackoverflow.com/questions/870150/how-to-access-static-resources-when-using-default-servlet/3593513#3593513 has an example of a better way to approach things if this is what you're trying to do.

Advice to remount DefaultServlet in Tomcat seems to have been around as long as Tomcat has existed, so perhaps we need to lock it down (so people can't accidentally create insecure configurations) or support mounting specific directories (inside or outside the web application), and break if accessing the root resources when mapped to a sub-path in any case.
Comment 5 Mark Thomas 2010-10-03 06:05:17 UTC
Some random thoughts:
- The default Servlet doesn't *need* to support serving content from an arbitrary mapping. The user can just move the static content in the web-app to the desired location
- The WebDAV Servlet (that extends the Default Servlet) does need to do this and I suspect is the reason for just using getPathInfo() and not getServletPath() + getPathInfo()
- We don't, currently, support configuring the Default Servlet for a sub-set of the application's URL space (without using a sub-context). Chuck's proposed change would support this. Need to think about the implications for WebDAV.

I'm leaning towards making Chuck's proposed change for the Default Servlet and over-riding that in the WebDAV Servlet to restore the current behaviour.
Comment 6 balusc 2010-10-03 11:59:03 UTC
@Tim: that was one of my answers over there :) This issue is by the way triggered by another Stackoverflow question (so it was not my intent to (ab)use the default servlet like that) http://stackoverflow.com/questions/3822524/tomcat-serving-static-content (I have also answered over there).

I personally can live with the WONTFIX, but this leaves a huge security hole for companies/ones who are using the DefaultServlet this way. This should probably better be documented in DefaultServlet's javadoc/manual.
Comment 7 Tim Whittington 2010-10-03 16:54:25 UTC
> I personally can live with the WONTFIX, but this leaves a huge security hole
> for companies/ones who are using the DefaultServlet this way. This should
> probably better be documented in DefaultServlet's javadoc/manual.

Yeah, I think closing the security issue is a must, given the prevalence of such use (the WONTFIX was more about the fact that this was an unintended use).

The DefaultServlet docs do need some tweaking (as well as WebdavServlet, which is almost anonymous) - will look at that as well.
Comment 8 Tim Whittington 2010-10-04 16:09:02 UTC
(In reply to comment #5)
> Some random thoughts:
> - The default Servlet doesn't *need* to support serving content from an
> arbitrary mapping. The user can just move the static content in the web-app to
> the desired location

Agreed, although it's a breaking change for people using it now.
I can't see the sense in re-mounting your entire webapp under a sub-path (except for WebDAV, where you want to sandbox the capabilities).

> - The WebDAV Servlet (that extends the Default Servlet) does need to do this
> and I suspect is the reason for just using getPathInfo() and not
> getServletPath() + getPathInfo()

The WebDAV Servlet already uses getPathInfo() explicitly, so won't be affected by this change.
It does suffer from exposing WEB-INF though, as it delegates GET requests to DefaultServlet.

> - We don't, currently, support configuring the Default Servlet for a sub-set of
> the application's URL space (without using a sub-context). Chuck's proposed
> change would support this. Need to think about the implications for WebDAV.

I can't see any major implications - I've tested with a combination of WebDAV mounted to subpath, WebDAV mounted to /*, and DefaultServlet mounted to / and a subpath, and all looks to work OK.
 
> I'm leaning towards making Chuck's proposed change for the Default Servlet and
> over-riding that in the WebDAV Servlet to restore the current behaviour.

+1, except that it's already overridden.

I'll commit some fixes to DefaultServlet and WebdavServlet, get some review and then propose changes for 6.0.x and perhaps 5.5.x.
Comment 9 Tim Whittington 2010-10-04 23:08:28 UTC
Fixes for DefaultServlet and WebdavServlet are committed for 7.0.x (will be in 7.0.4+) and proposed for 6.0.x.
Will need to check 5.5.x and see if a backport is required for that too.
Comment 10 Mark Thomas 2010-10-26 08:46:27 UTC
*** Bug 50153 has been marked as a duplicate of this bug. ***
Comment 11 bozho 2010-10-26 13:38:27 UTC
Instead of disallowing the whole remapping of the default servlet, can't this be implemented (as suggested) by using the getPathInfo(). for example:

String pathInfo = request.getPathInfo();
if (pathInfo.startsWith("WEB-INF") || pathInfo.startsWith("META-INF")) {
   // disallow, send 404
} else {
  proceed normally
}


This would mean that nothing existing would be broken, while the security hole will be fixed.

And there are actual reasons for remapping the default servlet. For example spring MVC prefers to have its servlet mapped to /, and static resources be served from a different path - say /static

I'm aware that this can be worked around by a Filter, or placing the static resources in a new app, or perhaps some more ways, but why should these be needed, when a simple mapping could do?
Comment 12 Chuck Caldarale 2010-10-26 14:21:24 UTC
(In reply to comment #11)
> Instead of disallowing the whole remapping of the default servlet, can't this
> be implemented (as suggested) by using the getPathInfo(). for example:
> 
> String pathInfo = request.getPathInfo();
> if (pathInfo.startsWith("WEB-INF") || pathInfo.startsWith("META-INF")) {
>    // disallow, send 404
> } else {
>   proceed normally
> }

Insufficient, since that would expose other directories that a site might not want to give direct access to.

> I'm aware that this can be worked around by a Filter, or placing the static
> resources in a new app, or perhaps some more ways, but why should these be
> needed, when a simple mapping could do?

Because the mapping is misleading, and numerous complaints have been lodged on the user's mailing list (and some here) about the previous discrepancy between the <url-pattern> and reality.

 - Chuck
Comment 13 bozho 2010-10-26 15:09:44 UTC
> Insufficient, since that would expose other directories that a site might not
> want to give direct access to.

For example? How would other directories be protected? 


> Because the mapping is misleading, and numerous complaints have been lodged on
> the user's mailing list (and some here) about the previous discrepancy between
> the <url-pattern> and reality.

Perhaps. To me, for example, it is entirely straightforward what's happening.
Comment 14 Christopher Schultz 2011-01-05 11:30:07 UTC
Has a CVE number been assigned to this? Seems there should be one, and this vulnerability should be documented in Tomcat's security page(s).
Comment 15 Mark Thomas 2011-01-05 11:48:05 UTC
No. The view was that it was mis-configuration rather than a vulnerability (the default servlet was never intended to be mapped to anything other than /).
Comment 16 Mark Thomas 2011-01-08 13:58:14 UTC
This has been fixed in 6.0.x and will be included in 6.0.30 onwards.