diff --git a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp index 9d389c986389..51eadd4242b5 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.hpp @@ -250,7 +250,14 @@ class U256Codec { BB_ASSERT_EQ(vec.size(), calc_num_fields()); if constexpr (IsAnyOf) { return static_cast(vec[0]); - } else if constexpr (IsAnyOf) { + } else if constexpr (IsAnyOf) { + BB_ASSERT_LT( + vec[0], uint256_t(bb::fr::modulus), "Non-canonical scalar field element: value >= fr::modulus"); + return static_cast(vec[0]); + } else if constexpr (IsAnyOf) { + BB_ASSERT_LT(vec[0], uint256_t(fq::modulus), "Non-canonical base field element: value >= fq::modulus"); + return static_cast(vec[0]); + } else if constexpr (IsAnyOf) { return static_cast(vec[0]); } else if constexpr (IsAnyOf) { using BaseField = typename T::Fq; diff --git a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.test.cpp b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.test.cpp index a3ad4c20d4b8..c2ae83f1fced 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/fields/field_conversion.test.cpp @@ -271,4 +271,88 @@ TEST_F(FieldConversionTest, RejectPointNotOnCurve) } #endif +// ============================================================================ +// U256Codec canonical checks +// ============================================================================ + +/** + * @brief Test that U256Codec accepts canonical field elements. + */ +TEST_F(FieldConversionTest, U256CodecAcceptsCanonicalFr) +{ + // Zero + { + std::vector vec = { uint256_t(0) }; + auto result = U256Codec::deserialize_from_fields(vec); + EXPECT_EQ(result, bb::fr(0)); + } + // One + { + std::vector vec = { uint256_t(1) }; + auto result = U256Codec::deserialize_from_fields(vec); + EXPECT_EQ(result, bb::fr(1)); + } + // modulus - 1 (largest canonical value) + { + std::vector vec = { uint256_t(bb::fr::modulus) - 1 }; + auto result = U256Codec::deserialize_from_fields(vec); + EXPECT_EQ(result, bb::fr(bb::fr::modulus - 1)); + } +} + +/** + * @brief Test that U256Codec accepts canonical fq elements. + */ +TEST_F(FieldConversionTest, U256CodecAcceptsCanonicalFq) +{ + using fq = grumpkin::fr; + std::vector vec = { uint256_t(0) }; + auto result = U256Codec::deserialize_from_fields(vec); + EXPECT_EQ(result, fq(0)); + + vec = { uint256_t(fq::modulus) - 1 }; + result = U256Codec::deserialize_from_fields(vec); + EXPECT_EQ(result, fq(fq::modulus - 1)); +} + +/** + * @brief Test that U256Codec rejects non-canonical field elements (>= modulus). + * @details Security-critical: prevents Fiat-Shamir challenge grinding in Keccak transcript. + */ +#ifndef __wasm__ +TEST_F(FieldConversionTest, U256CodecRejectsNonCanonicalFr) +{ + // fr::modulus itself (v + 0*p where v=0 but encoded as p) + { + std::vector vec = { uint256_t(bb::fr::modulus) }; + EXPECT_THROW_OR_ABORT(U256Codec::deserialize_from_fields(vec), "Non-canonical"); + } + // fr::modulus + 1 + { + std::vector vec = { uint256_t(bb::fr::modulus) + 1 }; + EXPECT_THROW_OR_ABORT(U256Codec::deserialize_from_fields(vec), "Non-canonical"); + } + // 2 * fr::modulus (another alias for 0) + { + std::vector vec = { uint256_t(bb::fr::modulus) * 2 }; + EXPECT_THROW_OR_ABORT(U256Codec::deserialize_from_fields(vec), "Non-canonical"); + } +} + +TEST_F(FieldConversionTest, U256CodecRejectsNonCanonicalFq) +{ + using fq = grumpkin::fr; + // fq::modulus itself + { + std::vector vec = { uint256_t(fq::modulus) }; + EXPECT_THROW_OR_ABORT(U256Codec::deserialize_from_fields(vec), "Non-canonical"); + } + // fq::modulus + 1 + { + std::vector vec = { uint256_t(fq::modulus) + 1 }; + EXPECT_THROW_OR_ABORT(U256Codec::deserialize_from_fields(vec), "Non-canonical"); + } +} +#endif + } // namespace bb::field_conversion_tests