bulletproofs: a few fixes from the Kudelski review

- fix integer overflow in n_bulletproof_amounts
- check input scalars are in range
- remove use of environment variable to tweak straus performance
- do not use implementation defined signed shift for signum

4 files changed, 23 insertions, 15 deletions

diff --git a/src/crypto/crypto-ops.c b/src/crypto/crypto-ops.c
index 45d412ac6..c1fff1d44 100644
--- a/src/crypto/crypto-ops.c
+++ b/src/crypto/crypto-ops.c
@@ -3707,9 +3707,8 @@ void sc_muladd(unsigned char *s, const unsigned char *a, const unsigned char *b,
   s[31] = s11 >> 17;
 }
 
-/* Assumes that a != INT64_MIN */
 static int64_t signum(int64_t a) {
-  return (a >> 63) - ((-a) >> 63);
+  return a > 0 ? 1 : a < 0 ? -1 : 0;
 }
 
 int sc_check(const unsigned char *s) {
diff --git a/src/ringct/bulletproofs.cc b/src/ringct/bulletproofs.cc
index 7ec87378b..d3822c454 100644
--- a/src/ringct/bulletproofs.cc
+++ b/src/ringct/bulletproofs.cc
@@ -74,14 +74,20 @@ static boost::mutex init_mutex;
 
 static inline rct::key multiexp(const std::vector<MultiexpData> &data, bool HiGi)
 {
-  static const size_t STEP = getenv("STRAUS_STEP") ? atoi(getenv("STRAUS_STEP")) : 0;
   if (HiGi)
   {
     static_assert(128 <= STRAUS_SIZE_LIMIT, "Straus in precalc mode can only be calculated till STRAUS_SIZE_LIMIT");
-    return data.size() <= 128 ? straus(data, straus_HiGi_cache, STEP) : pippenger(data, pippenger_HiGi_cache, get_pippenger_c(data.size()));
+    return data.size() <= 128 ? straus(data, straus_HiGi_cache, 0) : pippenger(data, pippenger_HiGi_cache, get_pippenger_c(data.size()));
   }
   else
-    return data.size() <= 64 ? straus(data, NULL, STEP) : pippenger(data, NULL, get_pippenger_c(data.size()));
+    return data.size() <= 64 ? straus(data, NULL, 0) : pippenger(data, NULL, get_pippenger_c(data.size()));
+}
+
+static bool is_reduced(const rct::key &scalar)
+{
+  rct::key reduced = scalar;
+  sc_reduce32(reduced.bytes);
+  return scalar == reduced;
 }
 
 //addKeys3acc_p3
@@ -659,6 +665,10 @@ Bulletproof bulletproof_PROVE(const rct::keyV &sv, const rct::keyV &gamma)
 {
   CHECK_AND_ASSERT_THROW_MES(sv.size() == gamma.size(), "Incompatible sizes of sv and gamma");
   CHECK_AND_ASSERT_THROW_MES(!sv.empty(), "sv is empty");
+  for (const rct::key &sve: sv)
+    CHECK_AND_ASSERT_THROW_MES(is_reduced(sve), "Invalid sv input");
+  for (const rct::key &g: gamma)
+    CHECK_AND_ASSERT_THROW_MES(is_reduced(g), "Invalid gamma input");
 
   init_exponents();
 
@@ -935,6 +945,13 @@ bool bulletproof_VERIFY(const std::vector<const Bulletproof*> &proofs)
     CHECK_AND_ASSERT_MES(rct::isInMainSubgroup(proof.T1), false, "Input point not in subgroup");
     CHECK_AND_ASSERT_MES(rct::isInMainSubgroup(proof.T2), false, "Input point not in subgroup");
 
+    // check scalar range
+    CHECK_AND_ASSERT_MES(is_reduced(proof.taux), false, "Input scalar not in range");
+    CHECK_AND_ASSERT_MES(is_reduced(proof.mu), false, "Input scalar not in range");
+    CHECK_AND_ASSERT_MES(is_reduced(proof.a), false, "Input scalar not in range");
+    CHECK_AND_ASSERT_MES(is_reduced(proof.b), false, "Input scalar not in range");
+    CHECK_AND_ASSERT_MES(is_reduced(proof.t), false, "Input scalar not in range");
+
     CHECK_AND_ASSERT_MES(proof.V.size() >= 1, false, "V does not have at least one element");
     CHECK_AND_ASSERT_MES(proof.L.size() == proof.R.size(), false, "Mismatched L and R sizes");
     CHECK_AND_ASSERT_MES(proof.L.size() > 0, false, "Empty proof");
diff --git a/src/ringct/rctTypes.cpp b/src/ringct/rctTypes.cpp
index e67637af6..5dd59aec3 100644
--- a/src/ringct/rctTypes.cpp
+++ b/src/ringct/rctTypes.cpp
@@ -236,6 +236,7 @@ namespace rct {
     size_t n_bulletproof_amounts(const Bulletproof &proof)
     {
         CHECK_AND_ASSERT_MES(proof.L.size() >= 6, 0, "Invalid bulletproof L size");
+        CHECK_AND_ASSERT_MES(proof.L.size() <= 31, 0, "Insane bulletproof L size");
         return 1 << (proof.L.size() - 6);
     }
 
@@ -245,6 +246,7 @@ namespace rct {
         for (const Bulletproof &proof: proofs)
         {
             size_t n2 = n_bulletproof_amounts(proof);
+            CHECK_AND_ASSERT_MES(n2 < std::numeric_limits<uint32_t>::max() - n, 0, "Invalid number of bulletproofs");
             if (n2 == 0)
                 return 0;
             n += n2;
diff --git a/tests/unit_tests/bulletproofs.cpp b/tests/unit_tests/bulletproofs.cpp
index 2358aa27a..e34982070 100644
--- a/tests/unit_tests/bulletproofs.cpp
+++ b/tests/unit_tests/bulletproofs.cpp
@@ -185,16 +185,6 @@ TEST(bulletproofs, invalid_gamma_0)
   ASSERT_FALSE(rct::bulletproof_VERIFY(proof));
 }
 
-TEST(bulletproofs, invalid_gamma_ff)
-{
-  rct::key invalid_amount = rct::zero();
-  invalid_amount[8] = 1;
-  rct::key gamma = rct::zero();
-  memset(&gamma, 0xff, sizeof(gamma));
-  rct::Bulletproof proof = bulletproof_PROVE(invalid_amount, gamma);
-  ASSERT_FALSE(rct::bulletproof_VERIFY(proof));
-}
-
 static const char * const torsion_elements[] =
 {
   "c7176a703d4dd84fba3c0b760d10670f2a2053fa2c39ccc64ec7fd7792ac03fa",