Enhance security (1) (neo-project#1403)

* inputs validation * more validation * recover constructor in ECPoint * format * minor changes * format * changes based on review * recover validation in AesEncrypt
Tommo-L · Jun 22, 2020 · 82f060b · 82f060b
1 parent 633384f
commit 82f060b
Show file tree

Hide file tree

Showing 9 changed files with 41 additions and 176 deletions.
diff --git a/src/neo/Cryptography/Base58.cs b/src/neo/Cryptography/Base58.cs
@@ -12,6 +12,7 @@ public static class Base58
 
         public static byte[] Base58CheckDecode(this string input)
         {
+            if (input is null) throw new ArgumentNullException(nameof(input));
             byte[] buffer = Decode(input);
             if (buffer.Length < 4) throw new FormatException();
             byte[] checksum = buffer.Sha256(0, buffer.Length - 4).Sha256();

diff --git a/src/neo/Cryptography/BloomFilter.cs b/src/neo/Cryptography/BloomFilter.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Collections;
 using System.Linq;
 
@@ -16,6 +17,7 @@ public class BloomFilter
 
         public BloomFilter(int m, int k, uint nTweak, byte[] elements = null)
         {
+            if (k < 0 || m < 0) throw new ArgumentOutOfRangeException();
             this.seeds = Enumerable.Range(0, k).Select(p => (uint)p * 0xFBA4C795 + nTweak).ToArray();
             this.bits = elements == null ? new BitArray(m) : new BitArray(elements);
             this.bits.Length = m;

diff --git a/src/neo/Cryptography/ECC/ECDsa.cs b/src/neo/Cryptography/ECC/ECDsa.cs
diff --git a/src/neo/Cryptography/ECC/ECFieldElement.cs b/src/neo/Cryptography/ECC/ECFieldElement.cs
@@ -10,6 +10,8 @@ internal class ECFieldElement : IComparable<ECFieldElement>, IEquatable<ECFieldE
 
         public ECFieldElement(BigInteger value, ECCurve curve)
         {
+            if (curve is null)
+                throw new ArgumentNullException(nameof(curve));
             if (value >= curve.Q)
                 throw new ArgumentException("x value too large in field element");
             this.Value = value;
@@ -19,6 +21,7 @@ public ECFieldElement(BigInteger value, ECCurve curve)
         public int CompareTo(ECFieldElement other)
         {
             if (ReferenceEquals(this, other)) return 0;
+            if (!curve.Equals(other.curve)) throw new InvalidOperationException("Invalid comparision for points with different curves");
             return Value.CompareTo(other.Value);
         }
 
@@ -35,7 +38,7 @@ public override bool Equals(object obj)
 
         public bool Equals(ECFieldElement other)
         {
-            return Value.Equals(other.Value);
+            return Value.Equals(other.Value) && curve.Equals(other.curve);
         }
 
         private static BigInteger[] FastLucasSequence(BigInteger p, BigInteger P, BigInteger Q, BigInteger k)

diff --git a/src/neo/Cryptography/ECC/ECPoint.cs b/src/neo/Cryptography/ECC/ECPoint.cs
@@ -22,7 +22,7 @@ public bool IsInfinity
 
         internal ECPoint(ECFieldElement x, ECFieldElement y, ECCurve curve)
         {
-            if ((x != null && y == null) || (x == null && y != null))
+            if ((x is null ^ y is null) || (curve is null))
                 throw new ArgumentException("Exactly one of the field elements is null");
             this.X = x;
             this.Y = y;
@@ -31,6 +31,7 @@ internal ECPoint(ECFieldElement x, ECFieldElement y, ECCurve curve)
 
         public int CompareTo(ECPoint other)
         {
+            if (!Curve.Equals(other.Curve)) throw new InvalidOperationException("Invalid comparision for points with different curves");
             if (ReferenceEquals(this, other)) return 0;
             int result = X.CompareTo(other.X);
             if (result != 0) return result;

diff --git a/src/neo/Cryptography/MerkleTree.cs b/src/neo/Cryptography/MerkleTree.cs
@@ -15,7 +15,7 @@ public class MerkleTree
 
         internal MerkleTree(UInt256[] hashes)
         {
-            if (hashes.Length == 0) throw new ArgumentException();
+            if (hashes is null || hashes.Length == 0) throw new ArgumentException();
             this.root = Build(hashes.Select(p => new MerkleTreeNode { Hash = p }).ToArray());
             int depth = 1;
             for (MerkleTreeNode i = root; i.LeftChild != null; i = i.LeftChild)

diff --git a/tests/neo.UnitTests/Cryptography/ECC/UT_ECDsa.cs b/tests/neo.UnitTests/Cryptography/ECC/UT_ECDsa.cs
diff --git a/tests/neo.UnitTests/Cryptography/ECC/UT_ECFieldElement.cs b/tests/neo.UnitTests/Cryptography/ECC/UT_ECFieldElement.cs
@@ -23,13 +23,37 @@ public void TestECFieldElementConstructor()
             action.Should().Throw<ArgumentException>();
         }
 
+        [TestMethod]
+        public void TestCompareTo()
+        {
+            ECFieldElement X1 = new ECFieldElement(new BigInteger(100), ECCurve.Secp256k1);
+            ECFieldElement Y1 = new ECFieldElement(new BigInteger(200), ECCurve.Secp256k1);
+            ECFieldElement X2 = new ECFieldElement(new BigInteger(300), ECCurve.Secp256k1);
+            ECFieldElement Y2 = new ECFieldElement(new BigInteger(400), ECCurve.Secp256k1);
+            ECFieldElement X3 = new ECFieldElement(new BigInteger(100), ECCurve.Secp256r1);
+            ECFieldElement Y3 = new ECFieldElement(new BigInteger(400), ECCurve.Secp256r1);
+            ECPoint point1 = new ECPoint(X1, Y1, ECCurve.Secp256k1);
+            ECPoint point2 = new ECPoint(X2, Y1, ECCurve.Secp256k1);
+            ECPoint point3 = new ECPoint(X1, Y2, ECCurve.Secp256k1);
+            ECPoint point4 = new ECPoint(X3, Y3, ECCurve.Secp256r1);
+
+            point1.CompareTo(point1).Should().Be(0);
+            point1.CompareTo(point2).Should().Be(-1);
+            point2.CompareTo(point1).Should().Be(1);
+            point1.CompareTo(point3).Should().Be(-1);
+            point3.CompareTo(point1).Should().Be(1);
+            Action action = () => point3.CompareTo(point4);
+            action.Should().Throw<InvalidOperationException>();
+        }
+
         [TestMethod]
         public void TestEquals()
         {
             BigInteger input = new BigInteger(100);
             object element = new ECFieldElement(input, ECCurve.Secp256k1);
             element.Equals(element).Should().BeTrue();
             element.Equals(1).Should().BeFalse();
+            element.Equals(new ECFieldElement(input, ECCurve.Secp256r1)).Should().BeFalse();
 
             input = new BigInteger(200);
             element.Equals(new ECFieldElement(input, ECCurve.Secp256k1)).Should().BeFalse();

diff --git a/tests/neo.UnitTests/Cryptography/ECC/UT_ECPoint.cs b/tests/neo.UnitTests/Cryptography/ECC/UT_ECPoint.cs
@@ -28,18 +28,12 @@ public static byte[] generatePrivateKey(int privateKeyLength)
         public void TestCompareTo()
         {
             ECFieldElement X1 = new ECFieldElement(new BigInteger(100), ECCurve.Secp256k1);
-            ECFieldElement Y1 = new ECFieldElement(new BigInteger(200), ECCurve.Secp256k1);
-            ECFieldElement X2 = new ECFieldElement(new BigInteger(300), ECCurve.Secp256k1);
-            ECFieldElement Y2 = new ECFieldElement(new BigInteger(400), ECCurve.Secp256k1);
-            ECPoint point1 = new ECPoint(X1, Y1, ECCurve.Secp256k1);
-            ECPoint point2 = new ECPoint(X2, Y1, ECCurve.Secp256k1);
-            ECPoint point3 = new ECPoint(X1, Y2, ECCurve.Secp256k1);
+            ECFieldElement X2 = new ECFieldElement(new BigInteger(200), ECCurve.Secp256k1);
+            ECFieldElement X3 = new ECFieldElement(new BigInteger(100), ECCurve.Secp256r1);
 
-            point1.CompareTo(point1).Should().Be(0);
-            point1.CompareTo(point2).Should().Be(-1);
-            point2.CompareTo(point1).Should().Be(1);
-            point1.CompareTo(point3).Should().Be(-1);
-            point3.CompareTo(point1).Should().Be(1);
+            X1.CompareTo(X2).Should().Be(-1);
+            Action action = () => X1.CompareTo(X3);
+            action.Should().Throw<InvalidOperationException>();
         }
 
         [TestMethod]
@@ -60,6 +54,8 @@ public void TestECPointConstructor()
             action.Should().Throw<ArgumentException>();
             action = () => new ECPoint(null, Y, ECCurve.Secp256k1);
             action.Should().Throw<ArgumentException>();
+            action = () => new ECPoint(null, Y, null);
+            action.Should().Throw<ArgumentException>();
         }
 
         [TestMethod]