Description

The bigfield::operator== function should constrain and return an in-circuit boolean indicating whether the two arguments are equal or not. The implementation of the function begins as follows:

template <typename Builder, typename T> bool_t<Builder> bigfield<Builder, T>::operator==(const bigfield& other) const
{
    Builder* ctx = context ? context : other.get_context();
    auto lhs = get_value() % modulus_u512;
    auto rhs = other.get_value() % modulus_u512;
    bool is_equal_raw = (lhs == rhs);
    if (!ctx) {
        // TODO(https://github.com/AztecProtocol/barretenberg/issues/660): null context _should_ mean that both are
        // constant, but we check with an assertion to be sure.
        ASSERT(is_constant() == other.is_constant());
        return is_equal_raw;
    }

Should both operands have null pointers as context, their values are compared directly and a constant with that boolean value is returned. This is only correct should both operands be constants. The comment points this out and mentions that while null contexts should only happen for constants, to be sure, it should be asserted that both operands are constant. However, the actual assert does not check this, but instead only checks that one operand is constant if and only if the other is. Thus, the case that both operands are not constants but still have a null pointer as context is not ruled out.

Impact

The assert intended to prevent this function being called with operands that have null-pointer contexts without being constants does not work as intended, so this situation is not prevented. However, this assert is intended as a defense-in-depth measure, so direct impact is unlikely.

Recommendations

Replace the assert with one that checks that both operands are constant:

-ASSERT(is_constant() == other.is_constant());
+ASSERT(is_constant() && other.is_constant());

Remediation

This issue has been acknowledged by Aztec, and a fix was implemented in commit c60705dd↗.