From e6b7c9626f7c8a028ff0074bc23016770bdb196f Mon Sep 17 00:00:00 2001 From: Alexander Filippov Date: Mon, 4 Dec 2023 11:35:22 +0300 Subject: [PATCH] Fix field elements validation --- .../org/tron/common/crypto/zksnark/BN128.java | 15 --------------- .../tron/common/crypto/zksnark/BN128Fp.java | 7 ++++++- .../tron/common/crypto/zksnark/BN128Fp2.java | 7 ++++++- .../org/tron/common/crypto/zksnark/Field.java | 2 -- .../org/tron/common/crypto/zksnark/Fp.java | 19 ++++++++++--------- .../org/tron/common/crypto/zksnark/Fp12.java | 5 ----- .../org/tron/common/crypto/zksnark/Fp2.java | 11 ++++++----- .../org/tron/common/crypto/zksnark/Fp6.java | 5 ----- 8 files changed, 28 insertions(+), 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 index 705b8601cee..2d465bb7018 100644 --- 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 index 4e3491759ab..ddfd1546972 100644 --- 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 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 create(byte[] xx, byte[] yy) { BN128 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 index 4a8e5d34a24..ca5157110cd 100644 --- 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 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 create(byte[] aa, byte[] bb, byte[] cc, byte[] dd) { BN128 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 index e4e34c4a13a..27b16e1740b 100644 --- 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 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 index ecb664f35d6..94ce2b595af 100644 --- 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 { } 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 index 29894332b6b..a09981c55ad 100644 --- 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 index 2654a337852..957068f9f75 100644 --- 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 index 0aee02cabd2..0680f88cd7a 100644 --- 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);