Fix field elements validation

zkBob · Dec 4, 2023 · e6b7c96 · e6b7c96
1 parent 575fe99
commit e6b7c96
Show file tree

Hide file tree

Showing 8 changed files with 28 additions and 43 deletions.
diff --git a/crypto/src/main/java/org/tron/common/crypto/zksnark/BN128.java b/crypto/src/main/java/org/tron/common/crypto/zksnark/BN128.java
@@ -220,21 +220,6 @@ public boolean isZero() {
     return z.isZero();
   }
 
-  protected boolean isValid() {
-
-    // check whether coordinates belongs to the Field
-    if (!x.isValid() || !y.isValid() || !z.isValid()) {
-      return false;
-    }
-
-    // check whether point is on the curve
-    if (!isOnCurve()) {
-      return false;
-    }
-
-    return true;
-  }
-
   @Override
   public String toString() {
     return String.format("(%s; %s; %s)", x.toString(), y.toString(), z.toString());

diff --git a/crypto/src/main/java/org/tron/common/crypto/zksnark/BN128Fp.java b/crypto/src/main/java/org/tron/common/crypto/zksnark/BN128Fp.java
@@ -47,6 +47,11 @@ public static BN128<Fp> create(byte[] xx, byte[] yy) {
     Fp x = Fp.create(xx);
     Fp y = Fp.create(yy);
 
+    if (x == null || y == null) {
+      // It means that one or both coordinates are not elements of Fp
+      return null;
+    }
+
     // check for point at infinity
     if (x.isZero() && y.isZero()) {
       return ZERO;
@@ -55,7 +60,7 @@ public static BN128<Fp> create(byte[] xx, byte[] yy) {
     BN128<Fp> p = new BN128Fp(x, y, Fp._1);
 
     // check whether point is a valid one
-    if (p.isValid()) {
+    if (p.isOnCurve()) {
       return p;
     } else {
       return null;

diff --git a/crypto/src/main/java/org/tron/common/crypto/zksnark/BN128Fp2.java b/crypto/src/main/java/org/tron/common/crypto/zksnark/BN128Fp2.java
@@ -52,6 +52,11 @@ public static BN128<Fp2> create(byte[] aa, byte[] bb, byte[] cc, byte[] dd) {
     Fp2 x = Fp2.create(aa, bb);
     Fp2 y = Fp2.create(cc, dd);
 
+    if (x == null || y == null) {
+      // It means that one or both coordinates are not elements of Fp
+      return null;
+    }
+
     // check for point at infinity
     if (x.isZero() && y.isZero()) {
       return ZERO;
@@ -60,7 +65,7 @@ public static BN128<Fp2> create(byte[] aa, byte[] bb, byte[] cc, byte[] dd) {
     BN128<Fp2> p = new BN128Fp2(x, y, Fp2._1);
 
     // check whether point is a valid one
-    if (p.isValid()) {
+    if (p.isOnCurve()) {
       return p;
     } else {
       return null;

diff --git a/crypto/src/main/java/org/tron/common/crypto/zksnark/Field.java b/crypto/src/main/java/org/tron/common/crypto/zksnark/Field.java
@@ -40,6 +40,4 @@ interface Field<T> {
   T negate();
 
   boolean isZero();
-
-  boolean isValid();
 }
diff --git a/crypto/src/main/java/org/tron/common/crypto/zksnark/Fp.java b/crypto/src/main/java/org/tron/common/crypto/zksnark/Fp.java
@@ -84,10 +84,19 @@ public class Fp implements Field<Fp> {
   }
 
   static Fp create(byte[] v) {
-    return new Fp(toMontgomery(new BigInteger(1, v)));
+    BigInteger value = new BigInteger(1, v);
+    if (value.compareTo(P) >= 0) {
+      // Only the values less than P are valid
+      return null;
+    }
+    return new Fp(toMontgomery(value));
   }
 
   static Fp create(BigInteger v) {
+    if (v.compareTo(P) >= 0) {
+      // Only the values less than P are valid
+      return null;
+    }
     return new Fp(toMontgomery(v));
   }
 
@@ -133,14 +142,6 @@ public boolean isZero() {
     return v.compareTo(BigInteger.ZERO) == 0;
   }
 
-  /**
-   * Checks if provided value is a valid Fp member
-   */
-  @Override
-  public boolean isValid() {
-    return v.compareTo(P) < 0;
-  }
-
   Fp2 mul(Fp2 o) {
     return new Fp2(o.a.mul(this), o.b.mul(this));
   }

diff --git a/crypto/src/main/java/org/tron/common/crypto/zksnark/Fp12.java b/crypto/src/main/java/org/tron/common/crypto/zksnark/Fp12.java
@@ -233,11 +233,6 @@ public boolean isZero() {
     return this.equals(ZERO);
   }
 
-  @Override
-  public boolean isValid() {
-    return a.isValid() && b.isValid();
-  }
-
   Fp12 frobeniusMap(int power) {
 
     Fp6 ra = a.frobeniusMap(power);

diff --git a/crypto/src/main/java/org/tron/common/crypto/zksnark/Fp2.java b/crypto/src/main/java/org/tron/common/crypto/zksnark/Fp2.java
@@ -60,6 +60,9 @@ static Fp2 create(BigInteger aa, BigInteger bb) {
 
     Fp a = Fp.create(aa);
     Fp b = Fp.create(bb);
+    if (a == null || b == null) {
+      return null;
+    }
 
     return new Fp2(a, b);
   }
@@ -68,6 +71,9 @@ static Fp2 create(byte[] aa, byte[] bb) {
 
     Fp a = Fp.create(aa);
     Fp b = Fp.create(bb);
+    if (a == null || b == null) {
+      return null;
+    }
 
     return new Fp2(a, b);
   }
@@ -139,11 +145,6 @@ public boolean isZero() {
     return this.equals(ZERO);
   }
 
-  @Override
-  public boolean isValid() {
-    return a.isValid() && b.isValid();
-  }
-
   @Override
   public boolean equals(Object o) {
     if (this == o) {

diff --git a/crypto/src/main/java/org/tron/common/crypto/zksnark/Fp6.java b/crypto/src/main/java/org/tron/common/crypto/zksnark/Fp6.java
@@ -211,11 +211,6 @@ public boolean isZero() {
     return this.equals(ZERO);
   }
 
-  @Override
-  public boolean isValid() {
-    return a.isValid() && b.isValid() && c.isValid();
-  }
-
   Fp6 frobeniusMap(int power) {
 
     Fp2 ra = a.frobeniusMap(power);