From 869b3bf824387ae50ab28be9bce66caae21bcae9 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Mon, 16 Jul 2018 14:40:51 +0100 Subject: [PATCH] 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 --- src/crypto/crypto-ops.c | 3 +-- src/ringct/bulletproofs.cc | 23 ++++++++++++++++++++--- src/ringct/rctTypes.cpp | 2 ++ tests/unit_tests/bulletproofs.cpp | 10 ---------- 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 &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 &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::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",