Skip to content

Commit

Permalink
Backport fix for CVE-2023-24998
Browse files Browse the repository at this point in the history
  • Loading branch information
jgallimore committed Feb 24, 2023
1 parent 05834ed commit b23d01c
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 18 deletions.
6 changes: 1 addition & 5 deletions build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1179,7 +1179,6 @@
debug="${compile.debug}" deprecation="${compile.deprecation}"
source="${compile.source}"
target="${compile.target}"
release="${compile.release}"
classpath="${tomcat.classes}"
encoding="ISO-8859-1"
includeantruntime="false">
Expand All @@ -1191,7 +1190,6 @@
debug="${compile.debug}" deprecation="${compile.deprecation}"
source="${compile.source}"
target="${compile.target}"
release="${compile.release}"
classpath="${tomcat.classes}"
encoding="ISO-8859-1"
includeantruntime="false">
Expand Down Expand Up @@ -1418,7 +1416,6 @@
deprecation="${compile.deprecation}"
source="${compile.source}"
target="${compile.target}"
release="${compile.release}"
encoding="ISO-8859-1"
includeantruntime="true">
<classpath refid="tomcat.test.classpath" />
Expand Down Expand Up @@ -1534,8 +1531,7 @@
errorproperty="test.result.error"
failureproperty="test.result.failure"
haltonfailure="${test.haltonfailure}"
jvm="${java.bin.path}java"
threads="${test.threads}">
jvm="${java.bin.path}java">

<!-- To enable remote debug for individual test-->
<!-- <jvmarg value="-Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=9009"/>-->
Expand Down
10 changes: 9 additions & 1 deletion java/org/apache/catalina/connector/Request.java
Original file line number Diff line number Diff line change
Expand Up @@ -2908,8 +2908,9 @@ private void parseParts() {
}
}

int maxParameterCount = getConnector().getMaxParameterCount();
Parameters parameters = coyoteRequest.getParameters();
parameters.setLimit(getConnector().getMaxParameterCount());
parameters.setLimit(maxParameterCount);

boolean success = false;
try {
Expand Down Expand Up @@ -2961,6 +2962,13 @@ private void parseParts() {
upload.setFileItemFactory(factory);
upload.setFileSizeMax(mce.getMaxFileSize());
upload.setSizeMax(mce.getMaxRequestSize());
if (maxParameterCount > -1) {
// There is a limit. The limit for parts needs to be reduced by
// the number of parameters we have already parsed.
// Must be under the limit else parsing parameters would have
// triggered an exception.
upload.setFileCountMax(maxParameterCount - parameters.size());
}

parts = new ArrayList<Part>();
try {
Expand Down
5 changes: 5 additions & 0 deletions java/org/apache/tomcat/util/http/Parameters.java
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ public void setParseFailedReason(FailReason failReason) {
}


public int size() {
return parameterCount;
}


public void recycle() {
parameterCount = 0;
paramHashValues.clear();
Expand Down
29 changes: 29 additions & 0 deletions java/org/apache/tomcat/util/http/fileupload/FileUploadBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Locale;
import java.util.Map;

import org.apache.tomcat.util.http.fileupload.impl.FileCountLimitExceededException;
import org.apache.tomcat.util.http.fileupload.impl.FileItemIteratorImpl;
import org.apache.tomcat.util.http.fileupload.impl.FileItemStreamImpl;
import org.apache.tomcat.util.http.fileupload.impl.FileUploadIOException;
Expand Down Expand Up @@ -133,6 +134,12 @@ public static final boolean isMultipartContent(RequestContext ctx) {
* to {@link #sizeMax}. A value of -1 indicates no maximum.
*/
private long fileSizeMax = -1;

/**
* The maximum permitted number of files that may be uploaded in a single
* request. A value of -1 indicates no maximum.
*/
private long fileCountMax = -1;

/**
* The content encoding to use when reading part headers.
Expand Down Expand Up @@ -209,6 +216,24 @@ public long getFileSizeMax() {
public void setFileSizeMax(long fileSizeMax) {
this.fileSizeMax = fileSizeMax;
}

/**
* Returns the maximum number of files allowed in a single request.
*
* @return The maximum number of files allowed in a single request.
*/
public long getFileCountMax() {
return fileCountMax;
}

/**
* Sets the maximum number of files allowed per request/
*
* @param fileCountMax The new limit. {@code -1} means no limit.
*/
public void setFileCountMax(long fileCountMax) {
this.fileCountMax = fileCountMax;
}

/**
* Retrieves the character encoding used when reading the headers of an
Expand Down Expand Up @@ -285,6 +310,10 @@ public List<FileItem> parseRequest(RequestContext ctx)
throw new NullPointerException("No FileItemFactory has been set.");
}
while (iter.hasNext()) {
if (items.size() == fileCountMax) {
// The next item will exceed the limit.
throw new FileCountLimitExceededException(ATTACHMENT, getFileCountMax());
}
final FileItemStream item = iter.next();
// Don't use getName() here to prevent an InvalidFileNameException.
final String fileName = ((FileItemStreamImpl) item).getName();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* 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.http.fileupload.impl;

import org.apache.tomcat.util.http.fileupload.FileUploadException;

/**
* This exception is thrown if a request contains more files than the specified
* limit.
*/
public class FileCountLimitExceededException extends FileUploadException {

private static final long serialVersionUID = 2408766352570556046L;

private final long limit;

/**
* Creates a new instance.
*
* @param message The detail message
* @param limit The limit that was exceeded
*/
public FileCountLimitExceededException(final String message, final long limit) {
super(message);
this.limit = limit;
}

/**
* Retrieves the limit that was exceeded.
*
* @return The limit that was exceeded by the request
*/
public long getLimit() {
return limit;
}
}
4 changes: 4 additions & 0 deletions webapps/docs/changelog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,10 @@
Shen, leeyazhou, winsonzhao, qingshi huang, Lay, Shucheng Hou and
Yanming Zhou. (markt)
</add>
<update>
Update the internal fork of Apache Commons FileUpload to aa8eff6
(2022-11-29, 2.0-SNAPSHOT). (markt)
</update>
</changelog>
</subsection>
</section>
Expand Down
15 changes: 9 additions & 6 deletions webapps/docs/config/ajp.xml
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,15 @@
</attribute>

<attribute name="maxParameterCount" required="false">
<p>The maximum number of parameter and value pairs (GET plus POST) which
will be automatically parsed by the container. Parameter and value pairs
beyond this limit will be ignored. A value of less than 0 means no limit.
If not specified, a default of 10000 is used. Note that
<code>FailedRequestFilter</code> <a href="filter.html">filter</a> can be
used to reject requests that hit the limit.</p>
<p>The maximum total number of request parameters (including uploaded
files) obtained from the query string and, for POST requests, the request
body if the content type is
<code>application/x-www-form-urlencoded</code> or
<code>multipart/form-data</code>. Request parameters beyond this limit
will be ignored. A value of less than 0 means no limit. If not specified,
a default of 10000 is used. Note that <code>FailedRequestFilter</code>
<a href="filter.html">filter</a> can be used to reject requests that
exceed the limit.</p>
</attribute>

<attribute name="maxPostSize" required="false">
Expand Down
15 changes: 9 additions & 6 deletions webapps/docs/config/http.xml
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,15 @@
</attribute>

<attribute name="maxParameterCount" required="false">
<p>The maximum number of parameter and value pairs (GET plus POST) which
will be automatically parsed by the container. Parameter and value pairs
beyond this limit will be ignored. A value of less than 0 means no limit.
If not specified, a default of 10000 is used. Note that
<code>FailedRequestFilter</code> <a href="filter.html">filter</a> can be
used to reject requests that hit the limit.</p>
<p>The maximum total number of request parameters (including uploaded
files) obtained from the query string and, for POST requests, the request
body if the content type is
<code>application/x-www-form-urlencoded</code> or
<code>multipart/form-data</code>. Request parameters beyond this limit
will be ignored. A value of less than 0 means no limit. If not specified,
a default of 10000 is used. Note that <code>FailedRequestFilter</code>
<a href="filter.html">filter</a> can be used to reject requests that
exceed the limit.</p>
</attribute>

<attribute name="maxPostSize" required="false">
Expand Down

0 comments on commit b23d01c

Please sign in to comment.