Env. prop.: Org.BouncyCastle.Asn1.AllowUnsafeInteger

- set to "true" to weaken ASN.1 INTEGER checks
- see https://github.com/bcgit/bc-csharp/issues/156

8 files changed, 387 insertions, 20 deletions

diff --git a/crypto/NBuild.build b/crypto/NBuild.build
index dd63df279..bbeb865a9 100644
--- a/crypto/NBuild.build
+++ b/crypto/NBuild.build
@@ -43,21 +43,21 @@
     <property name="switch" value="-" />
   </target>
   <target name="set-mono-2.0-framework-props">
-    <property name="compile-defines" value="NET_1_1" />
+    <property name="compile-defines" value="NET_2_0" />
     <property name="debug-extension" value="dll.mdb" />
     <property name="enable-nostdlib" value="false" />
     <property name="nunit-console" value="nunit-console" />
     <property name="switch" value="-" />
   </target>
   <target name="set-mono-3.5-framework-props">
-    <property name="compile-defines" value="NET_1_1" />
+    <property name="compile-defines" value="NET_2_0" />
     <property name="debug-extension" value="dll.mdb" />
     <property name="enable-nostdlib" value="false" />
     <property name="nunit-console" value="nunit-console" />
     <property name="switch" value="-" />
   </target>
   <target name="set-mono-4.0-framework-props">
-    <property name="compile-defines" value="NET_1_1" />
+    <property name="compile-defines" value="NET_2_0" />
     <property name="debug-extension" value="dll.mdb" />
     <property name="enable-nostdlib" value="false" />
     <property name="nunit-console" value="nunit-console" />
@@ -71,21 +71,21 @@
     <property name="switch" value="/" />
   </target>
   <target name="set-net-2.0-framework-props">
-    <property name="compile-defines" value="NET_1_1" />
+    <property name="compile-defines" value="NET_2_0" />
     <property name="debug-extension" value="pdb" />
     <property name="enable-nostdlib" value="true" />
     <property name="nunit-console" value="nunit-console.exe" />
     <property name="switch" value="/" />
   </target>
   <target name="set-net-3.5-framework-props">
-    <property name="compile-defines" value="NET_1_1" />
+    <property name="compile-defines" value="NET_2_0" />
     <property name="debug-extension" value="pdb" />
     <property name="enable-nostdlib" value="true" />
     <property name="nunit-console" value="nunit-console.exe" />
     <property name="switch" value="/" />
   </target>
   <target name="set-net-4.0-framework-props">
-    <property name="compile-defines" value="NET_1_1" />
+    <property name="compile-defines" value="NET_2_0" />
     <property name="debug-extension" value="pdb" />
     <property name="enable-nostdlib" value="true" />
     <property name="nunit-console" value="nunit-console.exe" />
diff --git a/crypto/crypto.csproj b/crypto/crypto.csproj
index bada122a3..b45530980 100644
--- a/crypto/crypto.csproj
+++ b/crypto/crypto.csproj
@@ -11195,6 +11195,11 @@
                     BuildAction = "Compile"
                 />
                 <File
+                    RelPath = "test\src\asn1\test\ASN1IntegerTest.cs"
+                    SubType = "Code"
+                    BuildAction = "Compile"
+                />
+                <File
                     RelPath = "test\src\asn1\test\ASN1SequenceParserTest.cs"
                     SubType = "Code"
                     BuildAction = "Compile"
diff --git a/crypto/src/asn1/DerEnumerated.cs b/crypto/src/asn1/DerEnumerated.cs
index db27065bb..6690feceb 100644
--- a/crypto/src/asn1/DerEnumerated.cs
+++ b/crypto/src/asn1/DerEnumerated.cs
@@ -62,19 +62,18 @@ namespace Org.BouncyCastle.Asn1
         }
 
         public DerEnumerated(
-            byte[]   bytes)
+            byte[] bytes)
         {
             if (bytes.Length > 1)
             {
-                if (bytes[0] == 0 && (bytes[1] & 0x80) == 0)
+                if ((bytes[0] == 0 && (bytes[1] & 0x80) == 0)
+                    || (bytes[0] == (byte)0xff && (bytes[1] & 0x80) != 0))
                 {
-                    throw new ArgumentException("malformed enumerated");
-                }
-                if (bytes[0] == (byte)0xff && (bytes[1] & 0x80) != 0)
-                {
-                    throw new ArgumentException("malformed enumerated");
+                    if (!DerInteger.AllowUnsafe())
+                        throw new ArgumentException("malformed enumerated");
                 }
             }
+
             this.bytes = Arrays.Clone(bytes);
         }
 
diff --git a/crypto/src/asn1/DerInteger.cs b/crypto/src/asn1/DerInteger.cs
index 5b240d281..ae14d2a9f 100644
--- a/crypto/src/asn1/DerInteger.cs
+++ b/crypto/src/asn1/DerInteger.cs
@@ -8,6 +8,14 @@ namespace Org.BouncyCastle.Asn1
     public class DerInteger
         : Asn1Object
     {
+        public const string AllowUnsafeProperty = "Org.BouncyCastle.Asn1.AllowUnsafeInteger";
+
+        internal static bool AllowUnsafe()
+        {
+            string allowUnsafeValue = Platform.GetEnvironmentVariable(AllowUnsafeProperty);
+            return allowUnsafeValue != null && Platform.EqualsIgnoreCase("true", allowUnsafeValue);
+        }
+
         private readonly byte[] bytes;
 
         /**
@@ -72,13 +80,11 @@ namespace Org.BouncyCastle.Asn1
         {
             if (bytes.Length > 1)
             {
-                if (bytes[0] == 0 && (bytes[1] & 0x80) == 0)
-                {
-                    throw new ArgumentException("malformed integer");
-                }
-                if (bytes[0] == (byte)0xff && (bytes[1] & 0x80) != 0)
+                if ((bytes[0] == 0 && (bytes[1] & 0x80) == 0)
+                    || (bytes[0] == (byte)0xff && (bytes[1] & 0x80) != 0))
                 {
-                    throw new ArgumentException("malformed integer");
+                    if (!AllowUnsafe())
+                        throw new ArgumentException("malformed integer");
                 }
             }
             this.bytes = Arrays.Clone(bytes);
diff --git a/crypto/src/util/Platform.cs b/crypto/src/util/Platform.cs
index 86484854d..6f7a8b17b 100644
--- a/crypto/src/util/Platform.cs
+++ b/crypto/src/util/Platform.cs
@@ -41,7 +41,7 @@ namespace Org.BouncyCastle.Utilities
 #endif
         }
 
-#if NETCF_1_0 || NETCF_2_0 || SILVERLIGHT || PORTABLE
+#if NETCF_1_0 || NETCF_2_0 || SILVERLIGHT || (PORTABLE && !DOTNET)
         internal static string GetEnvironmentVariable(
             string variable)
         {
diff --git a/crypto/test/UnitTests.csproj b/crypto/test/UnitTests.csproj
index d89cd6d56..2dd3547fa 100644
--- a/crypto/test/UnitTests.csproj
+++ b/crypto/test/UnitTests.csproj
@@ -51,6 +51,7 @@
     </ProjectReference>
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="src\asn1\test\ASN1IntegerTest.cs" />
     <Compile Include="src\asn1\test\ASN1SequenceParserTest.cs" />
     <Compile Include="src\asn1\test\ASN1UnitTest.cs" />
     <Compile Include="src\asn1\test\AdditionalInformationSyntaxUnitTest.cs" />
diff --git a/crypto/test/src/asn1/test/ASN1IntegerTest.cs b/crypto/test/src/asn1/test/ASN1IntegerTest.cs
new file mode 100644
index 000000000..6de3f7507
--- /dev/null
+++ b/crypto/test/src/asn1/test/ASN1IntegerTest.cs
@@ -0,0 +1,355 @@
+using System;
+
+using NUnit.Framework;
+
+using Org.BouncyCastle.Math;
+using Org.BouncyCastle.Utilities.Encoders;
+using Org.BouncyCastle.Utilities.Test;
+
+namespace Org.BouncyCastle.Asn1.Tests
+{
+    [TestFixture]
+    public class Asn1IntegerTest
+        : SimpleTest
+    {
+        private static readonly byte[] suspectKey = Base64.Decode(
+            "MIGJAoGBAHNc+iExm94LUrJdPSJ4QJ9tDRuvaNmGVHpJ4X7a5zKI02v+2E7RotuiR2MHDJfVJkb9LUs2kb3XBlyENhtMLsbeH+3Muy3" +
+            "hGDlh/mLJSh1s4c5jDKBRYOHom7Uc8wP0P2+zBCA+OEdikNDFBaP5PbR2Xq9okG2kPh35M2quAiMTAgMBAAE=");
+
+        public override string Name
+        {
+            get { return "Asn1Integer"; }
+        }
+
+        public override void PerformTest()
+        {
+#if NETCF_1_0 || NETCF_2_0 || SILVERLIGHT || (PORTABLE && !DOTNET) || NET_1_1
+            // Can't SetEnvironmentVariable !
+#else
+            SetAllowUnsafeProperty(true);
+
+            Asn1Sequence.GetInstance(suspectKey);
+
+            DoTestValidEncodingSingleByte();
+            DoTestValidEncodingMultiByte();
+            DoTestInvalidEncoding_00();
+            DoTestInvalidEncoding_ff();
+            DoTestInvalidEncoding_00_32bits();
+            DoTestInvalidEncoding_ff_32bits();
+            //DoDoTestLooseInvalidValidEncoding_FF_32B();
+            //DoTestLooseInvalidValidEncoding_zero_32B();
+            DoTestLooseValidEncoding_zero_32BAligned();
+            DoTestLooseValidEncoding_FF_32BAligned();
+            DoTestLooseValidEncoding_FF_32BAligned_1not0();
+            DoTestLooseValidEncoding_FF_32BAligned_2not0();
+            DoTestOversizedEncoding();
+
+            SetAllowUnsafeProperty(true);
+
+            new DerInteger(Hex.Decode("ffda47bfc776bcd269da4832626ac332adfca6dd835e8ecd83cd1ebe7d709b0e"));
+
+            new DerEnumerated(Hex.Decode("ffda47bfc776bcd269da4832626ac332adfca6dd835e8ecd83cd1ebe7d709b0e"));
+
+            SetAllowUnsafeProperty(false);
+
+            try
+            {
+                new DerInteger(Hex.Decode("ffda47bfc776bcd269da4832626ac332adfca6dd835e8ecd83cd1ebe7d709b"));
+
+                Fail("no exception");
+            }
+            catch (ArgumentException e)
+            {
+                IsEquals("malformed integer", e.Message);
+            }
+
+            // No support for thread-local override in C# version
+            //IsTrue(!Properties.SetThreadOverride("org.bouncycastle.asn1.allow_unsafe_integer", true));
+
+            //new DerInteger(Hex.Decode("ffda47bfc776bcd269da4832626ac332adfca6dd835e8ecd83cd1ebe7d709b"));
+
+            //IsTrue(Properties.RemoveThreadOverride("org.bouncycastle.asn1.allow_unsafe_integer"));
+
+            try
+            {
+                Asn1Sequence.GetInstance(suspectKey);
+
+                Fail("no exception");
+            }
+            catch (ArgumentException e)
+            {
+                IsEquals("test 1", "failed to construct sequence from byte[]: corrupted stream detected", e.Message);
+            }
+
+            try
+            {
+                new DerInteger(Hex.Decode("ffda47bfc776bcd269da4832626ac332adfca6dd835e8ecd83cd1ebe7d709b0e"));
+
+                Fail("no exception");
+            }
+            catch (ArgumentException e)
+            {
+                IsEquals("malformed integer", e.Message);
+            }
+
+            try
+            {
+                new DerEnumerated(Hex.Decode("ffda47bfc776bcd269da4832626ac332adfca6dd835e8ecd83cd1ebe7d709b0e"));
+
+                Fail("no exception");
+            }
+            catch (ArgumentException e)
+            {
+                IsEquals("malformed enumerated", e.Message);
+            }
+#endif
+        }
+
+        /**
+         * Ensure existing single byte behavior.
+         */
+        public void DoTestValidEncodingSingleByte()
+        {
+            SetAllowUnsafeProperty(false);
+
+            //
+            // Without property, single byte.
+            //
+            byte[] rawInt = Hex.Decode("10");
+            DerInteger i = new DerInteger(rawInt);
+            IsEquals(i.Value.IntValue, 16);
+
+            //
+            // With property set.
+            //
+            SetAllowUnsafeProperty(true);
+
+            rawInt = Hex.Decode("10");
+            i = new DerInteger(rawInt);
+            IsEquals(i.Value.IntValue, 16);
+        }
+
+        public void DoTestValidEncodingMultiByte()
+        {
+            SetAllowUnsafeProperty(false);
+
+            //
+            // Without property, multi-byte.
+            //
+            byte[] rawInt = Hex.Decode("10FF");
+            DerInteger i = new DerInteger(rawInt);
+            IsEquals(i.Value.IntValue, 4351);
+
+            //
+            // With property set.
+            //
+            SetAllowUnsafeProperty(true);
+
+            rawInt = Hex.Decode("10FF");
+            i = new DerInteger(rawInt);
+            IsEquals(i.Value.IntValue, 4351);
+        }
+
+        public void DoTestInvalidEncoding_00()
+        {
+            SetAllowUnsafeProperty(false);
+            try
+            {
+                byte[] rawInt = Hex.Decode("0010FF");
+                DerInteger i = new DerInteger(rawInt);
+                IsEquals(i.Value.IntValue, 4351);
+                Fail("Expecting illegal argument exception.");
+            }
+            catch (ArgumentException e)
+            {
+                IsEquals("malformed integer", e.Message);
+            }
+        }
+
+        public void DoTestInvalidEncoding_ff()
+        {
+            SetAllowUnsafeProperty(false);
+
+            try
+            {
+                byte[] rawInt = Hex.Decode("FF81FF");
+                DerInteger i = new DerInteger(rawInt);
+                Fail("Expecting illegal argument exception.");
+            }
+            catch (ArgumentException e)
+            {
+                IsEquals("malformed integer", e.Message);
+            }
+        }
+
+        public void DoTestInvalidEncoding_00_32bits()
+        {
+            SetAllowUnsafeProperty(false);
+
+            //
+            // Check what would pass loose validation fails outside of loose validation.
+            //
+            try
+            {
+                byte[] rawInt = Hex.Decode("0000000010FF");
+                DerInteger i = new DerInteger(rawInt);
+                IsEquals(i.Value.IntValue, 4351);
+                Fail("Expecting illegal argument exception.");
+            }
+            catch (ArgumentException e)
+            {
+                IsEquals("malformed integer", e.Message);
+            }
+        }
+
+        public void DoTestInvalidEncoding_ff_32bits()
+        {
+            SetAllowUnsafeProperty(false);
+
+            //
+            // Check what would pass loose validation fails outside of loose validation.
+            //
+            try
+            {
+                byte[] rawInt = Hex.Decode("FFFFFFFF01FF");
+                DerInteger i = new DerInteger(rawInt);
+                Fail("Expecting illegal argument exception.");
+            }
+            catch (ArgumentException e)
+            {
+                IsEquals("malformed integer", e.Message);
+            }
+        }
+
+        /*
+         Unfortunately it turns out that integers stored without sign bits that are assumed to be
+         unsigned.. this means a string of FF may occur and then the user will call getPositiveValue().
+         Sigh..
+        public void DoTestLooseInvalidValidEncoding_zero_32B()
+            throws Exception
+        {
+            //
+            // Should still fail as loose validation only permits 3 leading 0x00 bytes.
+            //
+            try
+            {
+                SetAllowUnsafeProperty(true);
+                byte[] rawInt = Hex.Decode("0000000010FF");
+                DerInteger i = new DerInteger(rawInt);
+                Fail("Expecting illegal argument exception.");
+            }
+            catch (ArgumentException e)
+            {
+                IsEquals("malformed integer", e.Message);
+            }
+        }
+
+        public void DoDoTestLooseInvalidValidEncoding_FF_32B()
+            throws Exception
+        {
+            //
+            // Should still fail as loose validation only permits 3 leading 0xFF bytes.
+            //
+            try
+            {
+                SetAllowUnsafeProperty(true);
+                byte[] rawInt = Hex.Decode("FFFFFFFF10FF");
+                DerInteger i = new DerInteger(rawInt);
+                Fail("Expecting illegal argument exception.");
+            }
+            catch (ArgumentException e)
+            {
+                IsEquals("malformed integer", e.Message);
+            }
+        }
+        */
+
+        public void DoTestLooseValidEncoding_zero_32BAligned()
+        {
+            //
+            // Should pass as loose validation permits 3 leading 0x00 bytes.
+            //
+            SetAllowUnsafeProperty(true);
+            byte[] rawInt = Hex.Decode("00000010FF000000");
+            DerInteger i = new DerInteger(rawInt);
+            IsEquals(72997666816L, i.Value.LongValue);
+        }
+
+        public void DoTestLooseValidEncoding_FF_32BAligned()
+        {
+            //
+            // Should pass as loose validation permits 3 leading 0xFF bytes
+            //
+            SetAllowUnsafeProperty(true);
+            byte[] rawInt = Hex.Decode("FFFFFF10FF000000");
+            DerInteger i = new DerInteger(rawInt);
+            IsEquals(-1026513960960L, i.Value.LongValue);
+        }
+
+        public void DoTestLooseValidEncoding_FF_32BAligned_1not0()
+        {
+            //
+            // Should pass as loose validation permits 3 leading 0xFF bytes.
+            //
+            SetAllowUnsafeProperty(true);
+            byte[] rawInt = Hex.Decode("FFFEFF10FF000000");
+            DerInteger i = new DerInteger(rawInt);
+            IsEquals(-282501490671616L, i.Value.LongValue);
+        }
+
+        public void DoTestLooseValidEncoding_FF_32BAligned_2not0()
+        {
+            //
+            // Should pass as loose validation permits 3 leading 0xFF bytes.
+            //
+            SetAllowUnsafeProperty(true);
+            byte[] rawInt = Hex.Decode("FFFFFE10FF000000");
+            DerInteger i = new DerInteger(rawInt);
+            IsEquals(-2126025588736L, i.Value.LongValue);
+        }
+
+        public void DoTestOversizedEncoding()
+        {
+            //
+            // Should pass as loose validation permits 3 leading 0xFF bytes.
+            //
+            SetAllowUnsafeProperty(true);
+            byte[] rawInt = Hex.Decode("FFFFFFFE10FF000000000000");
+            DerInteger i = new DerInteger(rawInt);
+            IsEquals(new BigInteger(Hex.Decode("FFFFFFFE10FF000000000000")), i.Value);
+
+            rawInt = Hex.Decode("FFFFFFFFFE10FF000000000000");
+            try
+            {
+                new DerInteger(rawInt);
+            }
+            catch (ArgumentException e)
+            {
+                IsEquals("malformed integer", e.Message);
+            }
+        }
+
+        private void SetAllowUnsafeProperty(bool allowUnsafe)
+        {
+#if NETCF_1_0 || NETCF_2_0 || SILVERLIGHT || (PORTABLE && !DOTNET) || NET_1_1
+            // Can't SetEnvironmentVariable !
+#else
+            Environment.SetEnvironmentVariable(DerInteger.AllowUnsafeProperty, allowUnsafe ? "true" : "false");
+#endif
+        }
+
+        public static void Main(
+            string[] args)
+        {
+            RunTest(new Asn1IntegerTest());
+        }
+
+        [Test]
+        public void TestFunction()
+        {
+            string resultText = Perform().ToString();
+
+            Assert.AreEqual(Name + ": Okay", resultText);
+        }
+    }
+}
diff --git a/crypto/test/src/asn1/test/RegressionTest.cs b/crypto/test/src/asn1/test/RegressionTest.cs
index eeca9ccd0..4534f2c75 100644
--- a/crypto/test/src/asn1/test/RegressionTest.cs
+++ b/crypto/test/src/asn1/test/RegressionTest.cs
@@ -11,6 +11,7 @@ namespace Org.BouncyCastle.Asn1.Tests
 			new AdditionalInformationSyntaxUnitTest(),
 			new AdmissionSyntaxUnitTest(),
 			new AdmissionsUnitTest(),
+            new Asn1IntegerTest(),
 			new AttributeTableUnitTest(),
             new BiometricDataUnitTest(),
             new BitStringTest(),