Bug 61542 - Apache Tomcat Remote Code Execution via JSP Upload bypass
Summary: Apache Tomcat Remote Code Execution via JSP Upload bypass
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.5.15
Hardware: PC All
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-20 06:38 UTC by xxlegend
Modified: 2020-01-16 12:00 UTC (History)
1 user (show)



Attachments
proposal to improve doc of the readonly flag (671 bytes, patch)
2017-09-22 11:20 UTC, Peter Stöckli
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description xxlegend 2017-09-20 06:38:18 UTC
Description:
When running on Windows with HTTP PUTs enabled (e.g. via setting the
readonly initialisation parameter of the Default to false) it was
possible to upload a JSP file to the server via a specially crafted
request. This JSP could then be requested and any code it contained
would be executed by the server.

the poc is like this:
PUT /1.jsp/ HTTP/1.1
Host: 192.168.3.103:8080
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Referer: http://192.168.3.103:8080/examples/
Accept-Encoding: gzip, deflate
Accept-Language: en-US,en;q=0.8,zh-CN;q=0.6,zh;q=0.4,zh-TW;q=0.2
Cookie: JSESSIONID=A27674F21B3308B4D893205FD2E2BF94
Connection: close
Content-Length: 26

<% out.println("hello");%>

it is the bypass for CVE-2017-12615
Comment 1 Mark Thomas 2017-09-20 12:06:16 UTC
This additional issue has been confirmed and CVE-2017-12617 has been allocated.
Comment 2 Remy Maucherat 2017-09-20 12:37:35 UTC
Hum, actually this looks like a File API issue. With the (correct) /1.jsp/ path input, (new File(name)).getPath() just strips the trailing '/', and of course getAbsolutePath, which is used for the safety net check, also does it. There's a problem there.

Restoring the BZ name since it's pointless.
Comment 3 Mark Thomas 2017-09-20 13:03:16 UTC
The File API certainly isn't helping.

When a file named '/test.jsp' exists '/test.jsp/' -> '/test.jsp' is surprising. Less so when it doesn't exist because it could be referring to a directory and both forms are valid for a directory.
Comment 4 Remy Maucherat 2017-09-20 13:17:28 UTC
Well, every time there's surprising normalization, it causes security issues so it's a big API mistake :) The normalization of the input path should only happen for getCanonicalPath, that's the whole point.

Of course, I probably knew about this behavior a while ago then since there's the '/' check for get. On the plus side the issue is not that serious (readonly needed) so it's not the end of the world.
Comment 5 Mark Thomas 2017-09-20 15:34:39 UTC
Maybe a better check would be that, given the path will already have been normalised, if the the absolute path ends with the given name.
Comment 6 Mark Thomas 2017-09-20 15:58:33 UTC
Nope. That will fail for directories where the trailing '/' is provided since it will have been removed from the absolute and canonical paths.
Comment 7 Peter Stöckli 2017-09-22 08:02:14 UTC
Isn't the mere existence of the readonly parameter also part of the problem?

https://tomcat.apache.org/tomcat-7.0-doc/default-servlet.html
It is currently documented as "Is this context "read only", so HTTP commands like PUT and DELETE are rejected? [true]"

But it holds more "surprises". IMHO this parameter should NEVER be set to false. Maybe it can be removed or the documentation of this parameter can be improved?
Comment 8 Remy Maucherat 2017-09-22 09:20:33 UTC
(In reply to Peter Stöckli from comment #7)
> Isn't the mere existence of the readonly parameter also part of the problem?
> 
> https://tomcat.apache.org/tomcat-7.0-doc/default-servlet.html
> It is currently documented as "Is this context "read only", so HTTP commands
> like PUT and DELETE are rejected? [true]"
> 
> But it holds more "surprises". IMHO this parameter should NEVER be set to
> false. Maybe it can be removed or the documentation of this parameter can be
> improved?

Have you ever heard of WebDAV ? Obviously if we were writing Tomcat today, we would never bother implementing it. Also obviously, nobody running a public server should enable it, secured or not. But it's not going to be removed either.
Comment 9 Mark Thomas 2017-09-22 09:23:09 UTC
Indeed. Lots of folks run Tomcat with WebDAV on internal sites. Hard-coding readonly to true is simply not an option.

Regarding better documentation, patches welcome.
Comment 10 Peter Stöckli 2017-09-22 11:20:52 UTC
Created attachment 35361 [details]
proposal to improve doc of the readonly flag

First of all: your work is greatly appreciated!
And I didn't know that Tomcat is also widely used as WebDAV server. So it makes sense to keep that option.

Attached is a patch that could help improve the documentation of the readonly flag.
Comment 11 Mark Thomas 2017-09-22 14:13:30 UTC
Fixed in:
- trunk for 9.0.0 onwards
- 8.5.x for 8.5.22 onwards
- 8.0.x for 8.0.47 onwards
- 7.0.x for 7.0.82 onwards

I'm on the fence regarding the suggested documentation change. If a sysadmin doesn't understand what enabling HTTP PUT and/or DELETE means I don't think any realistic amount of documentation is going to result in a correctly secured Tomcat instance.

Maybe what we need is a link to the security page from every setting called out in the security page. Something to ponder / discuss on the dev@ list.