You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/17 08:51:42 UTC
(tomcat) branch 11.0.x updated: Report the correct length in Asn1Parser.parseLength() error messages
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/11.0.x by this push:
new 378b80fc0a Report the correct length in Asn1Parser.parseLength() error messages
378b80fc0a is described below
commit 378b80fc0af016047c198669f603de013004f814
Author: remm <re...@apache.org>
AuthorDate: Sat Oct 17 10:46:21 2026 +0200
Report the correct length in Asn1Parser.parseLength() error messages
Based on code from PR#1090 submitted by Michael Osipov.
Add test produced using OpenCode.
---
java/org/apache/tomcat/util/buf/Asn1Parser.java | 47 ++++--
.../apache/tomcat/util/buf/LocalStrings.properties | 3 +
.../org/apache/tomcat/util/buf/TestAsn1Parser.java | 179 +++++++++++++++++++++
webapps/docs/changelog.xml | 8 +
4 files changed, 227 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/tomcat/util/buf/Asn1Parser.java b/java/org/apache/tomcat/util/buf/Asn1Parser.java
index 62a8153b35..a506099650 100644
--- a/java/org/apache/tomcat/util/buf/Asn1Parser.java
+++ b/java/org/apache/tomcat/util/buf/Asn1Parser.java
@@ -60,6 +60,12 @@ public class Asn1Parser {
*/
public static final int TAG_ATTRIBUTE_BASE = 0xA0;
+ /**
+ * The maximum number of subsequent octets that may be used to encode a length. X.690 8.1.3.5 c) prohibits the
+ * initial length octet value that would permit 127 subsequent octets, which restricts the count to this value.
+ */
+ private static final int MAX_LENGTH_OCTETS = 126;
+
private final byte[] source;
private int pos = 0;
@@ -173,27 +179,48 @@ public class Asn1Parser {
* Parses an ASN.1 length field.
*
* @return the parsed length value
+ *
+ * @throws IllegalArgumentException if the length uses a form that is not supported by this implementation, if the
+ * decoded length cannot be represented by this implementation, or if the decoded length exceeds the
+ * number of bytes remaining in the input data
*/
public int parseLength() {
int len = next();
if (len > 127) {
int bytes = len - 128;
- if (bytes > 4) {
- throw new IllegalArgumentException(sm.getString("asn1Parser.lengthInvalid", Integer.valueOf(-1),
- Integer.valueOf(source.length - pos)));
+ /*
+ * X.690 8.1.3.6.1 defines the single octet 0x80, which requests zero subsequent octets, as the indefinite
+ * length form. DER does not permit that form (X.690 10.1) and it is not supported here. Treating it as a
+ * length of zero would silently consume the identifier and desynchronise the parse, since the contents are
+ * terminated by end-of-contents octets (X.690 8.1.5) rather than by a length.
+ */
+ if (bytes == 0) {
+ throw new IllegalArgumentException(sm.getString("asn1Parser.indefiniteLength"));
+ }
+ /*
+ * X.690 8.1.3.5 c) prohibits the initial length octet value 0xFF, which is reserved for future extension,
+ * so the number of subsequent octets is limited to MAX_LENGTH_OCTETS. Any value within that range is
+ * accepted, including the non-minimal encodings that are permitted by BER (X.690 8.1.3.5, Note 2) even
+ * though DER requires the minimum number of octets.
+ */
+ if (bytes > MAX_LENGTH_OCTETS) {
+ throw new IllegalArgumentException(sm.getString("asn1Parser.lengthOctetsInvalid",
+ Integer.valueOf(bytes), Integer.valueOf(MAX_LENGTH_OCTETS)));
}
- len = 0;
+ byte[] lenBytes = new byte[bytes];
for (int i = 0; i < bytes; i++) {
- len = len << 8;
- len = len + next();
+ lenBytes[i] = (byte) next();
}
- if (len < 0) {
- throw new IllegalArgumentException(sm.getString("asn1Parser.lengthInvalid", Integer.valueOf(-1),
- Integer.valueOf(source.length - pos)));
+ // Length octets are always unsigned per X.690 8.1.3.5.
+ BigInteger bigLen = new BigInteger(1, lenBytes);
+ if (bigLen.compareTo(BigInteger.valueOf(Integer.MAX_VALUE)) > 0) {
+ throw new IllegalArgumentException(sm.getString("asn1Parser.lengthUnsupported", bigLen,
+ Integer.valueOf(Integer.MAX_VALUE)));
}
+ len = bigLen.intValue();
}
if (source.length - pos < len) {
- throw new IllegalArgumentException(sm.getString("asn1Parser.lengthInvalid", Integer.valueOf(-1),
+ throw new IllegalArgumentException(sm.getString("asn1Parser.lengthInvalid", Integer.valueOf(len),
Integer.valueOf(source.length - pos)));
}
diff --git a/java/org/apache/tomcat/util/buf/LocalStrings.properties b/java/org/apache/tomcat/util/buf/LocalStrings.properties
index ef39209ecd..6a9893cbde 100644
--- a/java/org/apache/tomcat/util/buf/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/buf/LocalStrings.properties
@@ -16,7 +16,10 @@
# Do not edit this file directly.
# To edit translations see: https://tomcat.apache.org/getinvolved.html#Translations
+asn1Parser.indefiniteLength=The indefinite length form is not supported
asn1Parser.lengthInvalid=Invalid length [{0}] bytes reported when the input data length is [{1}] bytes
+asn1Parser.lengthOctetsInvalid=Invalid number of length octets [{0}], the maximum permitted is [{1}]
+asn1Parser.lengthUnsupported=The decoded length [{0}] cannot be represented by this implementation which supports a maximum length of [{1}]
asn1Parser.tagMismatch=Expected to find value [{0}] but found value [{1}]
asn1Parser.truncatedData=Need [{0}] bytes but only [{1}] are available
diff --git a/test/org/apache/tomcat/util/buf/TestAsn1Parser.java b/test/org/apache/tomcat/util/buf/TestAsn1Parser.java
new file mode 100644
index 0000000000..50ca694d90
--- /dev/null
+++ b/test/org/apache/tomcat/util/buf/TestAsn1Parser.java
@@ -0,0 +1,179 @@
+/*
+ * 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.buf;
+
+import java.math.BigInteger;
+import java.util.Arrays;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.tomcat.util.res.StringManager;
+
+/**
+ * Test cases for {@link Asn1Parser}.
+ */
+public class TestAsn1Parser {
+
+ private static final StringManager sm = StringManager.getManager(Asn1Parser.class);
+
+ private static final int MAX_LENGTH_OCTETS = 126;
+
+
+ /*
+ * Note: parseLength() reads the length field directly, so the source arrays below start with the length identifier
+ * octet rather than with a tag. The values used where a message is checked are kept below 1000 because the
+ * MessageFormat used by StringManager applies locale specific grouping separators to numbers.
+ */
+
+ @Test
+ public void testShortFormLength() {
+ assertLength(new byte[] { 0x05, 1, 2, 3, 4, 5 }, 5);
+ }
+
+
+ @Test
+ public void testShortFormMaxLength() {
+ // The short form covers lengths up to 127
+ byte[] source = new byte[1 + 127];
+ source[0] = 0x7F;
+ assertLength(source, 127);
+ }
+
+
+ @Test
+ public void testLongFormLength() {
+ assertLength(new byte[] { (byte) 0x81, 0x05, 1, 2, 3, 4, 5 }, 5);
+ }
+
+
+ @Test
+ public void testLongFormZeroLength() {
+ // A non-minimal zero length is accepted: BER permits more octets than needed
+ assertLength(new byte[] { (byte) 0x82, 0x00, 0x00 }, 0);
+ }
+
+
+ @Test
+ public void testNonMinimalLengthMoreThanFourOctets() {
+ // 0x85 with a five octet length for the value 10. The limit of four length
+ // octets that this parser previously enforced rejected this encoding, but it
+ // is valid BER.
+ assertLength(new byte[] { (byte) 0x85, 0x00, 0x00, 0x00, 0x00, 0x0A, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }, 10);
+ }
+
+
+ @Test
+ public void testMaxPermittedLengthOctets() {
+ // 0xFE requests 126 subsequent octets, the maximum X.690 8.1.3.5 c) permits
+ byte[] source = new byte[1 + MAX_LENGTH_OCTETS];
+ source[0] = (byte) 0xFE;
+ assertLength(source, 0);
+ }
+
+
+ @Test
+ public void testReservedLengthOctetCount() {
+ // X.690 8.1.3.5 c) prohibits the identifier value 0xFF, which would request
+ // 127 subsequent octets
+ byte[] source = new byte[1 + 127];
+ source[0] = (byte) 0xFF;
+ assertLengthRejected(source, "asn1Parser.lengthOctetsInvalid", Integer.valueOf(127),
+ Integer.valueOf(MAX_LENGTH_OCTETS));
+ }
+
+
+ @Test
+ public void testIndefiniteLengthRejected() {
+ // X.690 8.1.3.6.1 defines 0x80 as the indefinite length form, which DER does
+ // not permit. Treating it as a length of zero, as this parser previously did,
+ // silently consumes the identifier and desynchronises the parse.
+ assertLengthRejected(new byte[] { (byte) 0x80 }, "asn1Parser.indefiniteLength");
+ }
+
+
+ @Test
+ public void testShortFormExceedsRemainingData() {
+ // The decoded length, not a placeholder, must be reported
+ assertLengthRejected(new byte[] { 0x0A, 1, 2, 3 }, "asn1Parser.lengthInvalid", Integer.valueOf(10),
+ Integer.valueOf(3));
+ }
+
+
+ @Test
+ public void testLongFormExceedsRemainingData() {
+ assertLengthRejected(new byte[] { (byte) 0x82, 0x00, (byte) 0xC8, 1, 2, 3 }, "asn1Parser.lengthInvalid",
+ Integer.valueOf(200), Integer.valueOf(3));
+ }
+
+
+ @Test
+ public void testLengthExactlyMaximumValue() {
+ // Integer.MAX_VALUE is representable, so this fails on the remaining data
+ // rather than as an unsupported length
+ byte[] source = new byte[] { (byte) 0x84, 0x7F, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF };
+ assertLengthRejected(source, "asn1Parser.lengthInvalid", Integer.valueOf(Integer.MAX_VALUE),
+ Integer.valueOf(0));
+ }
+
+
+ @Test
+ public void testLengthExceedingMaximumValue() {
+ byte[] source = new byte[] { (byte) 0x84, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF };
+ assertLengthRejected(source, "asn1Parser.lengthUnsupported", new BigInteger("4294967295"),
+ Integer.valueOf(Integer.MAX_VALUE));
+ }
+
+
+ @Test
+ public void testLengthOctetsTruncated() {
+ // The length identifier requests four octets but the input provides one. The
+ // input is truncated rather than the length invalid.
+ assertLengthRejected(new byte[] { (byte) 0x84, 0x00 }, "asn1Parser.truncatedData", Integer.valueOf(1),
+ Integer.valueOf(0));
+ }
+
+
+ @Test
+ public void testFullLengthReportsActualLength() {
+ byte[] source = new byte[] { 0x0A, 1, 2, 3 };
+ try {
+ new Asn1Parser(source).parseFullLength();
+ Assert.fail("Expected an IllegalArgumentException for " + Arrays.toString(source));
+ } catch (IllegalArgumentException e) {
+ Assert.assertEquals(sm.getString("asn1Parser.lengthInvalid", Integer.valueOf(10), Integer.valueOf(3)),
+ e.getMessage());
+ }
+ }
+
+
+ private static void assertLength(byte[] source, int expected) {
+ Assert.assertEquals("Unexpected length for " + Arrays.toString(source), expected,
+ new Asn1Parser(source).parseLength());
+ }
+
+
+ private static void assertLengthRejected(byte[] source, String key, Object... args) {
+ try {
+ new Asn1Parser(source).parseLength();
+ Assert.fail("Expected an IllegalArgumentException for " + Arrays.toString(source));
+ } catch (IllegalArgumentException e) {
+ Assert.assertEquals("Unexpected message for " + Arrays.toString(source), sm.getString(key, args),
+ e.getMessage());
+ }
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 83101c9a1d..237581d7f6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -391,6 +391,14 @@
thread pools with idle threads to create unnecessary threads.
(remm)
</fix>
+ <fix>
+ Correct <code>Asn1Parser.parseLength()</code> to report the actual decoded
+ length instead of a placeholder value when the length is valid but exceeds
+ the available input data, and to use a distinct, accurate message when a
+ length cannot be represented by this implementation due to Java's 32-bit
+ signed <code>int</code> limitation rather than misreporting it as an
+ invalid length. (michaelo)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org