From d3fd5f9580f7f9d5f2d63f86e8e08d22ad18654c Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Fri, 15 Oct 2021 01:18:46 +0700 Subject: ASN1InputStream updates from bc-java - improve tag validation - improve handling of long form definite-length --- crypto/src/asn1/Asn1InputStream.cs | 79 ++++++++++++++++------------ crypto/test/src/asn1/test/InputStreamTest.cs | 15 +++--- crypto/test/src/openssl/test/ReaderTest.cs | 10 ++-- 3 files changed, 59 insertions(+), 45 deletions(-) diff --git a/crypto/src/asn1/Asn1InputStream.cs b/crypto/src/asn1/Asn1InputStream.cs index 1791e5356..6bb6311e1 100644 --- a/crypto/src/asn1/Asn1InputStream.cs +++ b/crypto/src/asn1/Asn1InputStream.cs @@ -242,9 +242,7 @@ namespace Org.BouncyCastle.Asn1 get { return limit; } } - internal static int ReadTagNumber( - Stream s, - int tag) + internal static int ReadTagNumber(Stream s, int tag) { int tagNo = tag & 0x1f; @@ -256,23 +254,32 @@ namespace Org.BouncyCastle.Asn1 tagNo = 0; int b = s.ReadByte(); + if (b < 31) + { + if (b < 0) + throw new EndOfStreamException("EOF found inside tag value."); + + throw new IOException("corrupted stream - high tag number < 31 found"); + } // X.690-0207 8.1.2.4.2 // "c) bits 7 to 1 of the first subsequent octet shall not all be zero." - if ((b & 0x7f) == 0) // Note: -1 will pass + if ((b & 0x7f) == 0) throw new IOException("corrupted stream - invalid high tag number found"); - while ((b >= 0) && ((b & 0x80) != 0)) + while ((b & 0x80) != 0) { - tagNo |= (b & 0x7f); + if (((uint)tagNo >> 24) != 0U) + throw new IOException("Tag number more than 31 bits"); + + tagNo |= b & 0x7f; tagNo <<= 7; b = s.ReadByte(); + if (b < 0) + throw new EndOfStreamException("EOF found inside tag value."); } - if (b < 0) - throw new EndOfStreamException("EOF found inside tag value."); - - tagNo |= (b & 0x7f); + tagNo |= b & 0x7f; } return tagNo; @@ -281,37 +288,43 @@ namespace Org.BouncyCastle.Asn1 internal static int ReadLength(Stream s, int limit, bool isParsing) { int length = s.ReadByte(); + if (0U == ((uint)length >> 7)) + { + // definite-length short form + return length; + } + if (0x80 == length) + { + // indefinite-length + return -1; + } if (length < 0) + { throw new EndOfStreamException("EOF found when length expected"); - - if (length == 0x80) - return -1; // indefinite-length encoding - - if (length > 127) + } + if (0xFF == length) { - int size = length & 0x7f; - - // Note: The invalid long form "0xff" (see X.690 8.1.3.5c) will be caught here - if (size > 4) - throw new IOException("DER length more than 4 bytes: " + size); - - length = 0; - for (int i = 0; i < size; i++) - { - int next = s.ReadByte(); + throw new IOException("invalid long form definite-length 0xFF"); + } - if (next < 0) - throw new EndOfStreamException("EOF found reading length"); + int octetsCount = length & 0x7F, octetsPos = 0; - length = (length << 8) + next; - } + length = 0; + do + { + int octet = s.ReadByte(); + if (octet < 0) + throw new EndOfStreamException("EOF found reading length"); - if (length < 0) - throw new IOException("corrupted stream - negative length found"); + if (((uint)length >> 23) != 0U) + throw new IOException("long form definite-length more than 31 bits"); - if (length >= limit && !isParsing) // after all we must have read at least 1 byte - throw new IOException("corrupted stream - out of bounds length found: " + length + " >= " + limit); + length = (length << 8) + octet; } + while (++octetsPos < octetsCount); + + if (length >= limit && !isParsing) // after all we must have read at least 1 byte + throw new IOException("corrupted stream - out of bounds length found: " + length + " >= " + limit); return length; } diff --git a/crypto/test/src/asn1/test/InputStreamTest.cs b/crypto/test/src/asn1/test/InputStreamTest.cs index 4cfb304d1..32eafe27c 100644 --- a/crypto/test/src/asn1/test/InputStreamTest.cs +++ b/crypto/test/src/asn1/test/InputStreamTest.cs @@ -38,23 +38,24 @@ namespace Org.BouncyCastle.Asn1.Tests } catch (IOException e) { - if (!e.Message.StartsWith("DER length more than 4 bytes")) - { + if (!e.Message.Equals("invalid long form definite-length 0xFF")) + { Fail("wrong exception: " + e.Message); } } - aIn = new Asn1InputStream(negativeLength); + // NOTE: Not really a "negative" length, but 32 bits + aIn = new Asn1InputStream(negativeLength); - try - { + try + { aIn.ReadObject(); Fail("negative length not detected."); } catch (IOException e) { - if (!e.Message.Equals("corrupted stream - negative length found")) - { + if (!e.Message.Equals("long form definite-length more than 31 bits")) + { Fail("wrong exception: " + e.Message); } } diff --git a/crypto/test/src/openssl/test/ReaderTest.cs b/crypto/test/src/openssl/test/ReaderTest.cs index d0bb3661b..b8dff29d2 100644 --- a/crypto/test/src/openssl/test/ReaderTest.cs +++ b/crypto/test/src/openssl/test/ReaderTest.cs @@ -189,13 +189,13 @@ namespace Org.BouncyCastle.OpenSsl.Tests doDudPasswordTest("ef677", 1, "corrupted stream - out of bounds length found: 2087569732 >= 447"); doDudPasswordTest("800ce", 2, "unknown tag 26 encountered"); doDudPasswordTest("b6cd8", 3, "DEF length 81 object truncated by 56"); - doDudPasswordTest("28ce09", 4, "DEF length 110 object truncated by 28"); - doDudPasswordTest("2ac3b9", 5, "DER length more than 4 bytes: 11"); + doDudPasswordTest("28ce09", 4, "corrupted stream - high tag number < 31 found"); + doDudPasswordTest("2ac3b9", 5, "long form definite-length more than 31 bits"); doDudPasswordTest("2cba96", 6, "DEF length 100 object truncated by 35"); doDudPasswordTest("2e3354", 7, "DEF length 42 object truncated by 9"); - doDudPasswordTest("2f4142", 8, "DER length more than 4 bytes: 14"); - doDudPasswordTest("2fe9bb", 9, "DER length more than 4 bytes: 65"); - doDudPasswordTest("3ee7a8", 10, "DER length more than 4 bytes: 57"); + doDudPasswordTest("2f4142", 8, "long form definite-length more than 31 bits"); + doDudPasswordTest("2fe9bb", 9, "long form definite-length more than 31 bits"); + doDudPasswordTest("3ee7a8", 10, "long form definite-length more than 31 bits"); doDudPasswordTest("41af75", 11, "unknown tag 16 encountered"); doDudPasswordTest("1704a5", 12, "corrupted stream detected"); doDudPasswordTest("1c5822", 13, "extra data found after object"); -- cgit 1.4.1