summary refs log tree commit diff
diff options
context:
space:
mode:
-rw-r--r--crypto/src/asn1/Asn1InputStream.cs79
-rw-r--r--crypto/test/src/asn1/test/InputStreamTest.cs15
-rw-r--r--crypto/test/src/openssl/test/ReaderTest.cs10
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");