This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new 3c37221 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63579 3c37221 is described below commit 3c372211a8e449d47cf20cc39a6b8b0771d717fc Author: Mark Thomas <ma...@apache.org> AuthorDate: Tue Jul 23 10:38:40 2019 +0100 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63579 Reject malformed OPTIONS requests with a 400 response rather than a 500 --- .../apache/catalina/connector/CoyoteAdapter.java | 5 - .../connector/TestCoyoteAdapterRequestFuzzing.java | 105 +++++++++++++++++++++ webapps/docs/changelog.xml | 5 + 3 files changed, 110 insertions(+), 5 deletions(-) diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java b/java/org/apache/catalina/connector/CoyoteAdapter.java index 3c28c13..1cc445d 100644 --- a/java/org/apache/catalina/connector/CoyoteAdapter.java +++ b/java/org/apache/catalina/connector/CoyoteAdapter.java @@ -1138,11 +1138,6 @@ public class CoyoteAdapter implements Adapter { return false; } - // URL * is acceptable - if ((end - start == 1) && b[start] == (byte) '*') { - return true; - } - int pos = 0; int index = 0; diff --git a/test/org/apache/catalina/connector/TestCoyoteAdapterRequestFuzzing.java b/test/org/apache/catalina/connector/TestCoyoteAdapterRequestFuzzing.java new file mode 100644 index 0000000..160ff6c --- /dev/null +++ b/test/org/apache/catalina/connector/TestCoyoteAdapterRequestFuzzing.java @@ -0,0 +1,105 @@ +/* + * 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.connector; + +import java.io.File; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; + +import org.apache.catalina.Context; +import org.apache.catalina.servlets.DefaultServlet; +import org.apache.catalina.startup.SimpleHttpClient; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; + +/* + * Various requests, usually originating from fuzzing, that have triggered an + * incorrect response from Tomcat - usually a 500 response rather than a 400 + * response. + */ +@RunWith(Parameterized.class) +public class TestCoyoteAdapterRequestFuzzing extends TomcatBaseTest { + + @Parameterized.Parameters(name = "{index}: uri[{0}], host[{1}], expected[{2}]") + public static Collection<Object[]> parameters() { + List<Object[]> parameterSets = new ArrayList<>(); + + parameterSets.add(new Object[] { "/", "lÿ#", "400" } ); + parameterSets.add(new Object[] { "*;", "", "400" } ); + + return parameterSets; + } + + @Parameter(0) + public String uri; + + @Parameter(1) + public String host; + + @Parameter(2) + public String expected; + + + @Test + public void doTest() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + File appDir = new File("test/webapp"); + Context ctxt = tomcat.addContext("", appDir.getAbsolutePath()); + Tomcat.addServlet(ctxt, "default", DefaultServlet.class.getName()); + ctxt.addServletMappingDecoded("/", "default"); + + tomcat.start(); + + String request = + "GET " + uri + " HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: " + host + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF; + + Client client = new Client(tomcat.getConnector().getLocalPort()); + client.setRequest(new String[] {request}); + + client.connect(); + client.processRequest(); + + // Expected response + String line = client.getResponseLine(); + Assert.assertTrue(line, line.startsWith("HTTP/1.1 " + expected + " ")); + } + + + private static final class Client extends SimpleHttpClient { + + public Client(int port) { + setPort(port); + } + + @Override + public boolean isResponseBodyOK() { + // Response body varies. It is the response code that is of interest + // in these tests. + return true; + } + } +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 9cf2619..4d69f44 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -60,6 +60,11 @@ Discard large byte buffers allocated using setBufferSize when recycling the request. (remm) </fix> + <fix> + <bug>63579</bug>: Correct parsing of malformed OPTIONS requests and + reject them with a 400 response rather than triggering an internal error + that results in a 500 response. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org