Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=42181
A. Better handling of edge conditions in chunk header processing (BZ 42181)
B. Improve chunk header parsing. Properly ignore chunk-extension suffix, not trying to parse digits contained in it. Reject chunks whose header is incorrect.
git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc5.5.x/trunk@1392254 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/STATUS.txt b/STATUS.txt
index 4b3c20c..bae7a2f 100644
--- a/STATUS.txt
+++ b/STATUS.txt
@@ -28,15 +28,6 @@
PATCHES PROPOSED TO BACKPORT:
[ New proposals should be added at the end of the list ]
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=42181
- A. Better handling of edge conditions in chunk header processing (BZ 42181)
- B. Improve chunk header parsing. Properly ignore chunk-extension suffix,
- not trying to parse digits contained in it. Reject chunks whose header is
- incorrect. (backport of r423453)
- https://issues.apache.org/bugzilla/attachment.cgi?id=29295
- +1: kkolinko, markt, kfujino
- -1:
-
* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53830
Better handling of Manager.randomFile default value on Windows
https://issues.apache.org/bugzilla/attachment.cgi?id=29331
diff --git a/connectors/http11/src/java/org/apache/coyote/http11/Constants.java b/connectors/http11/src/java/org/apache/coyote/http11/Constants.java
index a963613..8455c32 100644
--- a/connectors/http11/src/java/org/apache/coyote/http11/Constants.java
+++ b/connectors/http11/src/java/org/apache/coyote/http11/Constants.java
@@ -85,6 +85,11 @@
*/
public static final byte COLON = (byte) ':';
+ /**
+ * SEMI_COLON.
+ */
+ public static final byte SEMI_COLON = (byte) ';';
+
/**
* 'A'.
diff --git a/connectors/http11/src/java/org/apache/coyote/http11/Http11AprProcessor.java b/connectors/http11/src/java/org/apache/coyote/http11/Http11AprProcessor.java
index ec43d48..c481d26 100644
--- a/connectors/http11/src/java/org/apache/coyote/http11/Http11AprProcessor.java
+++ b/connectors/http11/src/java/org/apache/coyote/http11/Http11AprProcessor.java
@@ -113,7 +113,7 @@
initializeFilters();
// Cause loading of HexUtils
- int foo = HexUtils.DEC[0];
+ HexUtils.getDec('0');
// Cause loading of FastHttpDateFormat
FastHttpDateFormat.getCurrentDate();
@@ -1461,7 +1461,7 @@
int port = 0;
int mult = 1;
for (int i = valueL - 1; i > colonPos; i--) {
- int charValue = HexUtils.DEC[(int) valueB[i + valueS]];
+ int charValue = HexUtils.getDec(valueB[i + valueS]);
if (charValue == -1) {
// Invalid character
error = true;
diff --git a/connectors/http11/src/java/org/apache/coyote/http11/Http11Processor.java b/connectors/http11/src/java/org/apache/coyote/http11/Http11Processor.java
index 17999cd..ec60c01 100644
--- a/connectors/http11/src/java/org/apache/coyote/http11/Http11Processor.java
+++ b/connectors/http11/src/java/org/apache/coyote/http11/Http11Processor.java
@@ -111,7 +111,7 @@
initializeFilters();
// Cause loading of HexUtils
- int foo = HexUtils.DEC[0];
+ HexUtils.getDec('0');
}
@@ -1422,7 +1422,7 @@
int port = 0;
int mult = 1;
for (int i = valueL - 1; i > colonPos; i--) {
- int charValue = HexUtils.DEC[(int) valueB[i + valueS]];
+ int charValue = HexUtils.getDec(valueB[i + valueS]);
if (charValue == -1) {
// Invalid character
error = true;
diff --git a/connectors/http11/src/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java b/connectors/http11/src/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
index 76558da..cab325e 100644
--- a/connectors/http11/src/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
+++ b/connectors/http11/src/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
@@ -28,7 +28,8 @@
import org.apache.coyote.http11.InputFilter;
/**
- * Chunked input filter.
+ * Chunked input filter. Parses chunked data according to
+ * <a href="http://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.6.1">http://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.6.1</a><br>
*
* @author Remy Maucherat
*/
@@ -128,7 +129,7 @@
if (remaining <= 0) {
if (!parseChunkHeader()) {
- throw new IOException("Invalid chunk");
+ throw new IOException("Invalid chunk header");
}
if (endChunk) {
parseEndChunk();
@@ -236,6 +237,14 @@
/**
* Parse the header of a chunk.
+ * A chunk header can look like one of the following:<br />
+ * A10CRLF<br />
+ * F23;chunk-extension to be ignoredCRLF
+ *
+ * <p>
+ * The letters before CRLF or ';' (whatever comes first) must be valid hex
+ * digits. We should not parse F23IAMGONNAMESSTHISUP34CRLF as a valid
+ * header according to the spec.
*/
protected boolean parseChunkHeader()
throws IOException {
@@ -243,6 +252,7 @@
int result = 0;
boolean eol = false;
boolean readDigit = false;
+ boolean trailer = false;
while (!eol) {
@@ -254,11 +264,19 @@
if (buf[pos] == Constants.CR) {
} else if (buf[pos] == Constants.LF) {
eol = true;
- } else {
- if (HexUtils.DEC[buf[pos]] != -1) {
+ } else if (buf[pos] == Constants.SEMI_COLON) {
+ trailer = true;
+ } else if (!trailer) {
+ //don't read data after the trailer
+ int charValue = HexUtils.getDec(buf[pos]);
+ if (charValue != -1) {
readDigit = true;
result *= 16;
- result += HexUtils.DEC[buf[pos]];
+ result += charValue;
+ } else {
+ //we shouldn't allow invalid, non hex characters
+ //in the chunked header
+ return false;
}
}
diff --git a/connectors/jk/java/org/apache/coyote/ajp/AjpAprProcessor.java b/connectors/jk/java/org/apache/coyote/ajp/AjpAprProcessor.java
index fa70319..527546a 100644
--- a/connectors/jk/java/org/apache/coyote/ajp/AjpAprProcessor.java
+++ b/connectors/jk/java/org/apache/coyote/ajp/AjpAprProcessor.java
@@ -103,7 +103,7 @@
outputBuffer = ByteBuffer.allocateDirect(packetSize * 2);
// Cause loading of HexUtils
- int foo = HexUtils.DEC[0];
+ HexUtils.getDec('0');
// Cause loading of HttpMessages
HttpMessages.getMessage(200);
@@ -935,7 +935,7 @@
int port = 0;
int mult = 1;
for (int i = valueL - 1; i > colonPos; i--) {
- int charValue = HexUtils.DEC[(int) valueB[i + valueS]];
+ int charValue = HexUtils.getDec(valueB[i + valueS]);
if (charValue == -1) {
// Invalid character
error = true;
diff --git a/connectors/jk/java/org/apache/jk/common/HandlerRequest.java b/connectors/jk/java/org/apache/jk/common/HandlerRequest.java
index 9701acf..4af7bbd 100644
--- a/connectors/jk/java/org/apache/jk/common/HandlerRequest.java
+++ b/connectors/jk/java/org/apache/jk/common/HandlerRequest.java
@@ -674,7 +674,7 @@
int port = 0;
int mult = 1;
for (int i = valueL - 1; i > colonPos; i--) {
- int charValue = HexUtils.DEC[(int) valueB[i + valueS]];
+ int charValue = HexUtils.getDec(valueB[i + valueS]);
if (charValue == -1) {
// Invalid character
throw new CharConversionException("Invalid char in port: " + valueB[i + valueS]);
diff --git a/connectors/util/java/org/apache/tomcat/util/buf/HexUtils.java b/connectors/util/java/org/apache/tomcat/util/buf/HexUtils.java
index 192988a..80637ab 100644
--- a/connectors/util/java/org/apache/tomcat/util/buf/HexUtils.java
+++ b/connectors/util/java/org/apache/tomcat/util/buf/HexUtils.java
@@ -36,6 +36,7 @@
/**
* Table for HEX to DEC byte translation.
+ * @deprecated Use {@link #getDec(int)}
*/
public static final int[] DEC = {
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
@@ -76,6 +77,15 @@
// --------------------------------------------------------- Static Methods
+ public static int getDec(int index){
+ try {
+ return DEC[index];
+ } catch (ArrayIndexOutOfBoundsException ex) {
+ return -1;
+ }
+ }
+
+
/**
* Convert a String of hexadecimal digits into the corresponding
* byte array by encoding each two hexadecimal digits as a byte.
@@ -154,21 +164,21 @@
// assert valid data
int len;
if(hex.length < 4 ) return 0;
- if( DEC[hex[0]]<0 )
+ if( getDec(hex[0])<0 )
throw new IllegalArgumentException(sm.getString("hexUtil.bad"));
- len = DEC[hex[0]];
+ len = getDec(hex[0]);
len = len << 4;
- if( DEC[hex[1]]<0 )
+ if( getDec(hex[1])<0 )
throw new IllegalArgumentException(sm.getString("hexUtil.bad"));
- len += DEC[hex[1]];
+ len += getDec(hex[1]);
len = len << 4;
- if( DEC[hex[2]]<0 )
+ if( getDec(hex[2])<0 )
throw new IllegalArgumentException(sm.getString("hexUtil.bad"));
- len += DEC[hex[2]];
+ len += getDec(hex[2]);
len = len << 4;
- if( DEC[hex[3]]<0 )
+ if( getDec(hex[3])<0 )
throw new IllegalArgumentException(sm.getString("hexUtil.bad"));
- len += DEC[hex[3]];
+ len += getDec(hex[3]);
return len;
}
diff --git a/container/catalina/src/share/org/apache/catalina/util/HexUtils.java b/container/catalina/src/share/org/apache/catalina/util/HexUtils.java
index 685e63b..e277fcd 100644
--- a/container/catalina/src/share/org/apache/catalina/util/HexUtils.java
+++ b/container/catalina/src/share/org/apache/catalina/util/HexUtils.java
@@ -30,7 +30,10 @@
public final class HexUtils {
// Code from Ajp11, from Apache's JServ
- // Table for HEX to DEC byte translation
+ /**
+ * Table for HEX to DEC byte translation.
+ * @deprecated Use {@link #getDec(int)}
+ */
public static final int[] DEC = {
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
@@ -59,6 +62,18 @@
StringManager.getManager("org.apache.catalina.util");
+ // --------------------------------------------------------- Static Methods
+
+
+ public static int getDec(int index){
+ try {
+ return DEC[index];
+ } catch (ArrayIndexOutOfBoundsException ex) {
+ return -1;
+ }
+ }
+
+
/**
* Convert a String of hexadecimal digits into the corresponding
* byte array by encoding each two hexadecimal digits as a byte.
@@ -137,21 +152,21 @@
// assert valid data
int len;
if(hex.length < 4 ) return 0;
- if( DEC[hex[0]]<0 )
+ if( getDec(hex[0])<0 )
throw new IllegalArgumentException(sm.getString("hexUtil.bad"));
- len = DEC[hex[0]];
+ len = getDec(hex[0]);
len = len << 4;
- if( DEC[hex[1]]<0 )
+ if( getDec(hex[1])<0 )
throw new IllegalArgumentException(sm.getString("hexUtil.bad"));
- len += DEC[hex[1]];
+ len += getDec(hex[1]);
len = len << 4;
- if( DEC[hex[2]]<0 )
+ if( getDec(hex[2])<0 )
throw new IllegalArgumentException(sm.getString("hexUtil.bad"));
- len += DEC[hex[2]];
+ len += getDec(hex[2]);
len = len << 4;
- if( DEC[hex[3]]<0 )
+ if( getDec(hex[3])<0 )
throw new IllegalArgumentException(sm.getString("hexUtil.bad"));
- len += DEC[hex[3]];
+ len += getDec(hex[3]);
return len;
}
diff --git a/container/webapps/docs/changelog.xml b/container/webapps/docs/changelog.xml
index 53bba36..fc9f848 100644
--- a/container/webapps/docs/changelog.xml
+++ b/container/webapps/docs/changelog.xml
@@ -82,6 +82,12 @@
<add>
Implement the maxHeaderCount for the HTTP connectors. (kkolinko)
</add>
+ <fix>
+ <bug>42181</bug>: Better handling of edge conditions in chunk header
+ processing. Improve chunk header parsing. Properly ignore
+ chunk-extension suffix, not trying to parse digits contained in it.
+ Reject chunks whose header is incorrect. (kkolinko)
+ </fix>
</changelog>
</subsection>
<subsection name="Webapps">
