From c50f201131f6b65876268bf046b15c44e279ff87 Mon Sep 17 00:00:00 2001 From: Cesar Hernandez Date: Wed, 15 Nov 2023 16:55:47 -0600 Subject: [PATCH 1/2] Backport c83fe47 to mitigate CVE-2023-45648 --- java/org/apache/coyote/http11/InternalAprInputBuffer.java | 2 +- java/org/apache/coyote/http11/InternalInputBuffer.java | 2 +- java/org/apache/coyote/http11/InternalNioInputBuffer.java | 2 +- .../apache/coyote/http11/filters/ChunkedInputFilter.java | 8 +++++++- .../apache/coyote/http11/filters/LocalStrings.properties | 3 ++- webapps/docs/changelog.xml | 3 +++ 6 files changed, 15 insertions(+), 5 deletions(-) diff --git a/java/org/apache/coyote/http11/InternalAprInputBuffer.java b/java/org/apache/coyote/http11/InternalAprInputBuffer.java index 6f153a9d6859..4e6a81aeac14 100644 --- a/java/org/apache/coyote/http11/InternalAprInputBuffer.java +++ b/java/org/apache/coyote/http11/InternalAprInputBuffer.java @@ -484,7 +484,7 @@ private boolean parseHeader() throws IOException { headers.removeHeader(headers.size() - 1); skipLine(lineStart, start); return true; - } else if (chr != Constants.HT && HttpParser.isControl(chr)) { + } else if (HttpParser.isControl(chr) && chr != Constants.HT) { // Invalid value // Delete the header (it will be the most recent one) headers.removeHeader(headers.size() - 1); diff --git a/java/org/apache/coyote/http11/InternalInputBuffer.java b/java/org/apache/coyote/http11/InternalInputBuffer.java index c305d3f17f28..2572e7d8dc33 100644 --- a/java/org/apache/coyote/http11/InternalInputBuffer.java +++ b/java/org/apache/coyote/http11/InternalInputBuffer.java @@ -435,7 +435,7 @@ private boolean parseHeader() throws IOException { // Invalid value - also need to delete header skipLine(lineStart, start, true); return true; - } else if (chr != Constants.HT && HttpParser.isControl(chr)) { + } else if (HttpParser.isControl(chr) && chr != Constants.HT) { // Invalid value - also need to delete header skipLine(lineStart, start, true); return true; diff --git a/java/org/apache/coyote/http11/InternalNioInputBuffer.java b/java/org/apache/coyote/http11/InternalNioInputBuffer.java index a0e087790ec9..3fc5c888f6d6 100644 --- a/java/org/apache/coyote/http11/InternalNioInputBuffer.java +++ b/java/org/apache/coyote/http11/InternalNioInputBuffer.java @@ -670,7 +670,7 @@ private HeaderParseStatus parseHeader() } else if (prevChr == Constants.CR) { // Invalid value - also need to delete header return skipLine(true); - } else if (chr != Constants.HT && HttpParser.isControl(chr)) { + } else if (HttpParser.isControl(chr) && chr != Constants.HT) { // Invalid value - also need to delete header return skipLine(true); } else if (chr == Constants.SP || chr == Constants.HT) { diff --git a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java index 05e76802c1d2..7f140bc30cd8 100644 --- a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java +++ b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java @@ -31,6 +31,7 @@ import org.apache.tomcat.util.buf.MessageBytes; import org.apache.tomcat.util.http.MimeHeaders; import org.apache.tomcat.util.res.StringManager; +import org.apache.tomcat.util.http.parser.HttpParser; /** * Chunked input filter. Parses chunked data according to @@ -500,6 +501,9 @@ private boolean parseHeader() throws IOException { if (chr == Constants.COLON) { colon = true; + } else if (!HttpParser.isToken(chr)) { + // Non-token characters are illegal in header names + throw new IOException(sm.getString("chunkedInputFilter.invalidTrailerHeaderName")); } else { trailingHeaders.append(chr); } @@ -561,7 +565,9 @@ private boolean parseHeader() throws IOException { if (chr == Constants.CR || chr == Constants.LF) { parseCRLF(true); eol = true; - } else if (chr == Constants.SP) { + } else if (HttpParser.isControl(chr) && chr != Constants.HT) { + throw new IOException(sm.getString("chunkedInputFilter.invalidTrailerHeaderValue")); + } else if (chr == Constants.SP || chr == Constants.HT) { trailingHeaders.append(chr); } else { trailingHeaders.append(chr); diff --git a/java/org/apache/coyote/http11/filters/LocalStrings.properties b/java/org/apache/coyote/http11/filters/LocalStrings.properties index c0dc9d1b10f7..65fdbf248d38 100644 --- a/java/org/apache/coyote/http11/filters/LocalStrings.properties +++ b/java/org/apache/coyote/http11/filters/LocalStrings.properties @@ -23,5 +23,6 @@ chunkedInputFilter.invalidCrlfNoData=Invalid end of line sequence (no data avail chunkedInputFilter.invalidHeader=Invalid chunk header chunkedInputFilter.maxExtension=maxExtensionSize exceeded chunkedInputFilter.maxTrailer=maxTrailerSize exceeded - +chunkedInputFilter.invalidTrailerHeaderName=Invalid trailer header name (non-token character in name) +chunkedInputFilter.invalidTrailerHeaderValue=Invalid trailer header value (control character in value) inputFilter.maxSwallow=maxSwallowSize exceeded diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 6c4d150c2ec2..7d374ad28bd8 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -154,6 +154,9 @@ Include the problematic data in the error message when reporting that the provided request line contains an invalid component. (markt) + + Align validation of HTTP trailer fields with standard fields. (markt) + From 21e0f69f4cf696703ff360a3c92aa573ee6a2a35 Mon Sep 17 00:00:00 2001 From: Cesar Hernandez Date: Wed, 15 Nov 2023 17:08:09 -0600 Subject: [PATCH 2/2] Backport 30f8063 to mitigate CVE-2023-42795 --- .../catalina/connector/LocalStrings.properties | 1 + java/org/apache/catalina/connector/Request.java | 7 ++++--- .../catalina/core/ApplicationHttpRequest.java | 8 +++++++- .../apache/catalina/core/LocalStrings.properties | 1 + .../apache/catalina/core/LocalStrings_es.properties | 1 + .../apache/catalina/core/LocalStrings_fr.properties | 1 + .../apache/catalina/core/LocalStrings_ja.properties | 1 + .../apache/catalina/core/LocalStrings_ko.properties | 1 + .../catalina/core/LocalStrings_zh_CN.properties | 1 + java/org/apache/tomcat/util/buf/B2CConverter.java | 11 ++++++++++- java/org/apache/tomcat/util/buf/C2BConverter.java | 13 ++++++++++++- .../apache/tomcat/util/buf/LocalStrings.properties | 2 ++ webapps/docs/changelog.xml | 4 ++++ 13 files changed, 46 insertions(+), 6 deletions(-) diff --git a/java/org/apache/catalina/connector/LocalStrings.properties b/java/org/apache/catalina/connector/LocalStrings.properties index cef14110b397..3cb608c373bf 100644 --- a/java/org/apache/catalina/connector/LocalStrings.properties +++ b/java/org/apache/catalina/connector/LocalStrings.properties @@ -46,6 +46,7 @@ coyoteRequest.attributeEvent=Exception thrown by attributes event listener coyoteRequest.authenticate.ise=Cannot call authenticate() after the response has been committed coyoteRequest.changeSessionId=Cannot change session ID. There is no session associated with this request. coyoteRequest.chunkedPostTooLarge=Parameters were not parsed because the size of the posted data was too big. Because this request was a chunked request, it could not be processed further. Use the maxPostSize attribute of the connector to resolve this if the application should accept large POSTs. +coyoteRequest.deletePartFailed=Failed to deleted temporary file used for part [{0}] coyoteRequest.getContextPath.ise=Unable to find match between the canonical context path [{0}] and the URI presented by the user agent [{1}] coyoteRequest.getInputStream.ise=getReader() has already been called for this request coyoteRequest.getReader.ise=getInputStream() has already been called for this request diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java index 2c998a20379e..66a6b86c4515 100644 --- a/java/org/apache/catalina/connector/Request.java +++ b/java/org/apache/catalina/connector/Request.java @@ -484,8 +484,9 @@ public void recycle() { for (Part part: parts) { try { part.delete(); - } catch (IOException ignored) { - // ApplicationPart.delete() never throws an IOEx + } catch (Throwable t) { + ExceptionUtils.handleThrowable(t); + log.warn(sm.getString("coyoteRequest.deletePartFailed", part.getName()), t); } } parts = null; @@ -536,8 +537,8 @@ public void recycle() { asyncSupported = null; if (asyncContext!=null) { asyncContext.recycle(); + asyncContext = null; } - asyncContext = null; pathParameters.clear(); } diff --git a/java/org/apache/catalina/core/ApplicationHttpRequest.java b/java/org/apache/catalina/core/ApplicationHttpRequest.java index 99feca76784d..839907ac29d6 100644 --- a/java/org/apache/catalina/core/ApplicationHttpRequest.java +++ b/java/org/apache/catalina/core/ApplicationHttpRequest.java @@ -45,6 +45,7 @@ import org.apache.tomcat.util.buf.MessageBytes; import org.apache.tomcat.util.http.Parameters; import org.apache.tomcat.util.res.StringManager; +import org.apache.tomcat.util.ExceptionUtils; /** @@ -645,7 +646,12 @@ public boolean isRequestedSessionIdValid() { */ public void recycle() { if (session != null) { - session.endAccess(); + try { + session.endAccess(); + } catch (Throwable t) { + ExceptionUtils.handleThrowable(t); + context.getLogger().warn(sm.getString("applicationHttpRequest.sessionEndAccessFail"), t); + } } } diff --git a/java/org/apache/catalina/core/LocalStrings.properties b/java/org/apache/catalina/core/LocalStrings.properties index 7798b9ef91cf..05906b73aa5a 100644 --- a/java/org/apache/catalina/core/LocalStrings.properties +++ b/java/org/apache/catalina/core/LocalStrings.properties @@ -58,6 +58,7 @@ applicationFilterRegistration.nullInitParam=Unable to set initialisation paramet applicationFilterRegistration.nullInitParams=Unable to set initialisation parameters for filter due to null name and/or value. Name [{0}], Value [{1}] applicationHttpRequest.fragmentInDispatchPath=The fragment in dispatch path [{0}] has been removed +applicationHttpRequest.sessionEndAccessFail=Exception triggered ending access to session while recycling request applicationRequest.badParent=Cannot locate parent Request implementation applicationRequest.badRequest=Request is not a javax.servlet.ServletRequestWrapper diff --git a/java/org/apache/catalina/core/LocalStrings_es.properties b/java/org/apache/catalina/core/LocalStrings_es.properties index 8d0ce4284718..822939320e35 100644 --- a/java/org/apache/catalina/core/LocalStrings_es.properties +++ b/java/org/apache/catalina/core/LocalStrings_es.properties @@ -50,6 +50,7 @@ applicationFilterConfig.jmxUnregisterFail=Ha fallado el desregistro JMX para el applicationFilterRegistration.nullInitParam=No puedo poner el parámetro de inicialización para el filtro debido a un nombre nulo y/o valor. Nombre [{0}], Valor [{1}] applicationFilterRegistration.nullInitParams=No puedo poner los parámetros de inicialización para el filtro debido a un nombre nulo y/o valor. Nombre [{0}], Valor [{1}] +applicationHttpRequest.sessionEndAccessFail=Excepcin disparada acabando acceso a sesin mientras se reciclaba el requerimiento applicationRequest.badParent=No puedo localizar la implementación de Requerimiento padre applicationRequest.badRequest=El requerimiento no es un javax.servlet.ServletRequestWrapper diff --git a/java/org/apache/catalina/core/LocalStrings_fr.properties b/java/org/apache/catalina/core/LocalStrings_fr.properties index 92704d46e671..52df70d5b634 100644 --- a/java/org/apache/catalina/core/LocalStrings_fr.properties +++ b/java/org/apache/catalina/core/LocalStrings_fr.properties @@ -55,6 +55,7 @@ applicationFilterConfig.release=Impossible de détruite le filtre nommé [{0}] d applicationFilterRegistration.nullInitParam=Impossible de fixer le paramètre d''initialisation du filtre, à cause d''un nom ou d''une valeur nulle, nom [{0}], valeur [{1}] applicationFilterRegistration.nullInitParams=Impossible de fixer les paramètres d''initialisation du filtre, à cause d''un nom ou d''une valeur nulle, nom [{0}], valeur [{1}] +applicationHttpRequest.sessionEndAccessFail=Exception lance durant l'arrt de l'accs la session durant le recyclage de la requte applicationHttpRequest.fragmentInDispatchPath=Le fragment dans le chemin de dispatch [{0}] a été enlevé diff --git a/java/org/apache/catalina/core/LocalStrings_ja.properties b/java/org/apache/catalina/core/LocalStrings_ja.properties index 924684f1ef87..181f28a8ad3f 100644 --- a/java/org/apache/catalina/core/LocalStrings_ja.properties +++ b/java/org/apache/catalina/core/LocalStrings_ja.properties @@ -55,6 +55,7 @@ applicationFilterConfig.release=タイプ[{1}]の名前[{0}]のフィルタを applicationFilterRegistration.nullInitParam=NULLの名前や値のためにフィルターの初期化パラメーターを設定できません。 名前[{0}]、値[{1}] applicationFilterRegistration.nullInitParams=キー [{0}] または値 [{1}] のいずれかが null のためフィルターの初期化パラメータを設定できませんでした。 +applicationHttpRequest.sessionEndAccessFail=Exception triggered ending access to session while recycling request applicationHttpRequest.fragmentInDispatchPath=ディスパッチパス [{0}] 中のフラグメントは除去されました diff --git a/java/org/apache/catalina/core/LocalStrings_ko.properties b/java/org/apache/catalina/core/LocalStrings_ko.properties index ad737f65cc84..d8681f2a4ebe 100644 --- a/java/org/apache/catalina/core/LocalStrings_ko.properties +++ b/java/org/apache/catalina/core/LocalStrings_ko.properties @@ -53,6 +53,7 @@ applicationFilterConfig.release=타입이 [{1}](이)고 이름이 [{0}]인 필 applicationFilterRegistration.nullInitParam=이름 또는 값 또는 둘 다 널이어서, 필터를 위한 초기화 파라미터를 설정할 수 없습니다. 이름: [{0}], 값: [{1}] applicationFilterRegistration.nullInitParams=널인 이름 또는 값 때문에, 필터의 초기화 파라미터를 설정할 수 없습니다. 이름: [{0}], 값: [{1}] +applicationHttpRequest.sessionEndAccessFail=Exception triggered ending access to session while recycling request applicationHttpRequest.fragmentInDispatchPath=디스패치 경로 [{0}](으)로부터 URI fragment를 제거했습니다. diff --git a/java/org/apache/catalina/core/LocalStrings_zh_CN.properties b/java/org/apache/catalina/core/LocalStrings_zh_CN.properties index 79779a57511d..d7b06c382723 100644 --- a/java/org/apache/catalina/core/LocalStrings_zh_CN.properties +++ b/java/org/apache/catalina/core/LocalStrings_zh_CN.properties @@ -54,6 +54,7 @@ applicationFilterConfig.release=失败的销毁过滤器类型为[{1}]名称为[ applicationFilterRegistration.nullInitParam=由于名称和/或值为空，无法为筛选器设置初始化参数。名称[{0}]，值[{1}] applicationFilterRegistration.nullInitParams=由于name和(或)value为null，无法为过滤器设置初始化参数。name为 [{0}]，value为 [{1}] +applicationHttpRequest.sessionEndAccessFail=Exception triggered ending access to session while recycling request applicationHttpRequest.fragmentInDispatchPath=调度路径[{0}]中的片段已被删除 diff --git a/java/org/apache/tomcat/util/buf/B2CConverter.java b/java/org/apache/tomcat/util/buf/B2CConverter.java index 0b58d3f38216..f822a6e421ad 100644 --- a/java/org/apache/tomcat/util/buf/B2CConverter.java +++ b/java/org/apache/tomcat/util/buf/B2CConverter.java @@ -25,6 +25,9 @@ import java.nio.charset.CoderResult; import java.nio.charset.CodingErrorAction; import java.util.Locale; +import org.apache.juli.logging.Log; +import org.apache.juli.logging.LogFactory; +import org.apache.tomcat.util.ExceptionUtils; import org.apache.tomcat.util.res.StringManager; @@ -33,6 +36,7 @@ */ public class B2CConverter { + private static final Log log = LogFactory.getLog(B2CConverter.class); private static final StringManager sm = StringManager.getManager(Constants.Package); @@ -133,7 +137,12 @@ public B2CConverter(String encoding, boolean replaceOnError) * Reset the decoder state. */ public void recycle() { - decoder.reset(); + try { + decoder.reset(); + } catch (Throwable t) { + ExceptionUtils.handleThrowable(t); + log.warn(sm.getString("b2cConverter.decoderResetFail", decoder.charset()), t); + } leftovers.position(0); } diff --git a/java/org/apache/tomcat/util/buf/C2BConverter.java b/java/org/apache/tomcat/util/buf/C2BConverter.java index b57d47a46f49..e3e2b75ab24a 100644 --- a/java/org/apache/tomcat/util/buf/C2BConverter.java +++ b/java/org/apache/tomcat/util/buf/C2BConverter.java @@ -22,12 +22,18 @@ import java.nio.charset.CharsetEncoder; import java.nio.charset.CoderResult; import java.nio.charset.CodingErrorAction; +import org.apache.juli.logging.Log; +import org.apache.juli.logging.LogFactory; +import org.apache.tomcat.util.ExceptionUtils; +import org.apache.tomcat.util.res.StringManager; /** * NIO based character encoder. */ public final class C2BConverter { + private static final Log log = LogFactory.getLog(C2BConverter.class); + private static final StringManager sm = StringManager.getManager(C2BConverter.class); CharsetEncoder encoder = null; ByteBuffer bb = null; CharBuffer cb = null; @@ -51,7 +57,12 @@ public C2BConverter(String encoding) throws IOException { * Reset the encoder state. */ public void recycle() { - encoder.reset(); + try { + encoder.reset(); + } catch (Throwable t) { + ExceptionUtils.handleThrowable(t); + log.warn(sm.getString("c2bConverter.decoderResetFail", encoder.charset()), t); + } leftovers.position(0); } diff --git a/java/org/apache/tomcat/util/buf/LocalStrings.properties b/java/org/apache/tomcat/util/buf/LocalStrings.properties index 76859b5d64f5..f9cf77607650 100644 --- a/java/org/apache/tomcat/util/buf/LocalStrings.properties +++ b/java/org/apache/tomcat/util/buf/LocalStrings.properties @@ -14,8 +14,10 @@ # limitations under the License. b2cConverter.unknownEncoding=The character encoding [{0}] is not supported +b2cConverter.decoderResetFail=Failed to reset instance of decoder for character set [{0}] c2bConverter.recycleFailed=Failed to recycle the C2B Converter. Creating new BufferedWriter, WriteConvertor and IntermediateOutputStream. +c2bConverter.encoderResetFail=Failed to reset instance of encoder for character set [{0}] encodedSolidusHandling.invalid=The value [{0}] is not recognised diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 7d374ad28bd8..de225c3cf25b 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -128,6 +128,10 @@ RemoteIpFilter determines that this request was submitted via a secure channel. (lihan) + + Improve handling of failures within recycle() methods. + (markt) +