File tomcat-8.0.43-CVE-2017-12617.patch of Package tomcat.5927
Index: java/org/apache/tomcat/util/compat/JrePlatform.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- java/org/apache/tomcat/util/compat/JrePlatform.java (revision )
+++ java/org/apache/tomcat/util/compat/JrePlatform.java (revision )
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.compat;
+
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+
+public class JrePlatform {
+
+ private static final String OS_NAME_PROPERTY = "os.name";
+ private static final String OS_NAME_WINDOWS_PREFIX = "Windows";
+
+ static {
+ /*
+ * There are a few places where a) the behaviour of the Java API depends
+ * on the underlying platform and b) those behavioural differences have
+ * an impact on Tomcat.
+ *
+ * Tomcat therefore needs to be able to determine the platform it is
+ * running on to account for those differences.
+ *
+ * In an ideal world this code would not exist.
+ */
+
+ // This check is derived from the check in Apache Commons Lang
+ String osName;
+ if (System.getSecurityManager() == null) {
+ osName = System.getProperty(OS_NAME_PROPERTY);
+ } else {
+ osName = AccessController.doPrivileged(
+ new PrivilegedAction<String>() {
+
+ @Override
+ public String run() {
+ return System.getProperty(OS_NAME_PROPERTY);
+ }
+ });
+ }
+
+ IS_WINDOWS = osName.startsWith(OS_NAME_WINDOWS_PREFIX);
+ }
+
+
+ public static final boolean IS_WINDOWS;
+}
Index: test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java (revision )
+++ test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java (revision )
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.webresources;
+
+import java.util.regex.Pattern;
+
+import org.junit.Test;
+
+public class TestAbstractFileResourceSetPerformance {
+
+ private static final Pattern UNSAFE_WINDOWS_FILENAME_PATTERN = Pattern.compile(" $|[\"<>]");
+
+ private static final int LOOPS = 10_000_000;
+
+ /*
+ * Checking individual characters is about 10 times faster on markt's dev
+ * PC for typical length file names. The file names need to get to ~65
+ * characters before the Pattern matching is faster.
+ */
+ @Test
+ public void testFileNameFiltering() {
+
+ long start = System.nanoTime();
+ for (int i = 0; i < LOOPS; i++) {
+ UNSAFE_WINDOWS_FILENAME_PATTERN.matcher("testfile.jsp ").find();
+ }
+ long end = System.nanoTime();
+ System.out.println("Regular expression took " + (end - start) + "ns or " +
+ (end-start)/LOOPS + "ns per iteration");
+
+ start = System.nanoTime();
+ for (int i = 0; i < LOOPS; i++) {
+ checkForBadCharsArray("testfile.jsp ");
+ }
+ end = System.nanoTime();
+ System.out.println("char[] check took " + (end - start) + "ns or " +
+ (end-start)/LOOPS + "ns per iteration");
+
+ start = System.nanoTime();
+ for (int i = 0; i < LOOPS; i++) {
+ checkForBadCharsAt("testfile.jsp ");
+ }
+ end = System.nanoTime();
+ System.out.println("charAt() check took " + (end - start) + "ns or " +
+ (end-start)/LOOPS + "ns per iteration");
+
+ }
+
+ private boolean checkForBadCharsArray(String filename) {
+ char[] chars = filename.toCharArray();
+ for (char c : chars) {
+ if (c == '\"' || c == '<' || c == '>') {
+ return false;
+ }
+ }
+ if (chars[chars.length -1] == ' ') {
+ return false;
+ }
+ return true;
+ }
+
+
+ private boolean checkForBadCharsAt(String filename) {
+ final int len = filename.length();
+ for (int i = 0; i < len; i++) {
+ char c = filename.charAt(i);
+ if (c == '\"' || c == '<' || c == '>') {
+ return false;
+ }
+ }
+ if (filename.charAt(len - 1) == ' ') {
+ return false;
+ }
+ return true;
+ }
+}
Index: java/org/apache/catalina/webresources/AbstractFileResourceSet.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- java/org/apache/catalina/webresources/AbstractFileResourceSet.java (date 1490711901000)
+++ java/org/apache/catalina/webresources/AbstractFileResourceSet.java (revision )
@@ -22,6 +22,7 @@
import java.net.URL;
import org.apache.catalina.LifecycleException;
+import org.apache.tomcat.util.compat.JrePlatform;
import org.apache.tomcat.util.http.RequestUtil;
public abstract class AbstractFileResourceSet extends AbstractResourceSet {
@@ -57,53 +58,113 @@
name = "";
}
File file = new File(fileBase, name);
- if (!mustExist || file.canRead()) {
- if (getRoot().getAllowLinking()) {
- return file;
- }
+ // If the requested names ends in '/', the Java File API will return a
+ // matching file if one exists. This isn't what we want as it is not
+ // consistent with the Servlet spec rules for request mapping.
+ if (name.endsWith("/") && file.isFile()) {
+ return null;
+ }
+
+ // If the file/dir must exist but the identified file/dir can't be read
+ // then signal that the resource was not found
+ if (mustExist && !file.canRead()) {
+ return null;
+ }
+
+ // If allow linking is enabled, files are not limited to being located
+ // under the fileBase so all further checks are disabled.
+ if (getRoot().getAllowLinking()) {
+ return file;
+ }
- // Check that this file is located under the WebResourceSet's base
- String canPath = null;
- try {
- canPath = file.getCanonicalPath();
- } catch (IOException e) {
- // Ignore
- }
- if (canPath == null)
- return null;
+ // Additional Windows specific checks to handle known problems with
+ // File.getCanonicalPath()
+ if (JrePlatform.IS_WINDOWS && isInvalidWindowsFilename(name)) {
+ return null;
+ }
+
+ // Check that this file is located under the WebResourceSet's base
+ String canPath = null;
+ try {
+ canPath = file.getCanonicalPath();
+ } catch (IOException e) {
+ // Ignore
+ }
+ if (canPath == null || !canPath.startsWith(canonicalBase)) {
+ return null;
+ }
- if (!canPath.startsWith(canonicalBase)) {
- return null;
- }
+ // Ensure that the file is not outside the fileBase. This should not be
+ // possible for standard requests (the request is normalized early in
+ // the request processing) but might be possible for some access via the
+ // Servlet API (RequestDispatcher, HTTP/2 push etc.) therefore these
+ // checks are retained as an additional safety measure
+ // absoluteBase has been normalized so absPath needs to be normalized as
+ // well.
+ String absPath = normalize(file.getAbsolutePath());
+ if (absoluteBase.length() > absPath.length()) {
+ return null;
+ }
- // Case sensitivity check
- // Note: We know the resource is located somewhere under base at
- // point. The purpose of this code is to check in a case
- // sensitive manner, the path to the resource under base
- // agrees with what was requested
- String fileAbsPath = file.getAbsolutePath();
- if (fileAbsPath.endsWith("."))
- fileAbsPath = fileAbsPath + '/';
- String absPath = normalize(fileAbsPath);
- if ((absoluteBase.length() < absPath.length())
- && (canonicalBase.length() < canPath.length())) {
- absPath = absPath.substring(absoluteBase.length() + 1);
- if (absPath.equals(""))
- absPath = "/";
- canPath = canPath.substring(canonicalBase.length() + 1);
- if (canPath.equals(""))
- canPath = "/";
- if (!canPath.equals(absPath))
- return null;
- }
+ // Remove the fileBase location from the start of the paths since that
+ // was not part of the requested path and the remaining check only
+ // applies to the request path
+ absPath = absPath.substring(absoluteBase.length());
+ canPath = canPath.substring(canonicalBase.length());
+
+ // Case sensitivity check
+ // The normalized requested path should be an exact match the equivalent
+ // canonical path. If it is not, possible reasons include:
+ // - case differences on case insensitive file systems
+ // - Windows removing a trailing ' ' or '.' from the file name
+ //
+ // In all cases, a mis-match here results in the resource not being
+ // found
+ //
+ // absPath is normalized so canPath needs to be normalized as well
+ // Can't normalize canPath earlier as canonicalBase is not normalized
+ if (canPath.length() > 0) {
+ canPath = normalize(canPath);
+ }
+ if (!canPath.equals(absPath)) {
+ return null;
+ }
- } else {
- return null;
- }
return file;
}
+
+ private boolean isInvalidWindowsFilename(String name) {
+ final int len = name.length();
+ if (len == 0) {
+ return false;
+ }
+ // This consistently ~10 times faster than the equivalent regular
+ // expression irrespective of input length.
+ for (int i = 0; i < len; i++) {
+ char c = name.charAt(i);
+ if (c == '\"' || c == '<' || c == '>') {
+ // These characters are disallowed in Windows file names and
+ // there are known problems for file names with these characters
+ // when using File#getCanonicalPath().
+ // Note: There are additional characters that are disallowed in
+ // Windows file names but these are not known to cause
+ // problems when using File#getCanonicalPath().
+ return true;
+ }
+ }
+ // Windows does not allow file names to end in ' ' unless specific low
+ // level APIs are used to create the files that bypass various checks.
+ // File names that end in ' ' are known to cause problems when using
+ // File#getCanonicalPath().
+ if (name.charAt(len -1) == ' ') {
+ return true;
+ }
+ return false;
+ }
+
+
/**
* Return a context-relative path, beginning with a "/", that represents
* the canonical version of the specified path after ".." and "." elements
@@ -114,7 +175,7 @@
* @param path Path to be normalized
*/
private String normalize(String path) {
- return RequestUtil.normalize(path, File.separatorChar == '/');
+ return RequestUtil.normalize(path, File.separatorChar == '\\');
}
@Override
@@ -144,11 +205,7 @@
fileBase = new File(getBase(), getInternalPath());
checkType(fileBase);
- String absolutePath = fileBase.getAbsolutePath();
- if (absolutePath.endsWith(".")) {
- absolutePath = absolutePath + '/';
- }
- this.absoluteBase = normalize(absolutePath);
+ this.absoluteBase = normalize(fileBase.getAbsolutePath());
try {
this.canonicalBase = fileBase.getCanonicalPath();
