Description

The Limb struct is a wrapper around a circuit variable that also tracks that variable's maximum possible value with an out-of-circuit uint256_t. The constructor is implemented as follows:

Limb(const field_t<Builder>& input, const uint256_t max = uint256_t(0))
    : element(input)
{
    if (input.witness_index == IS_CONSTANT) {
        maximum_value = uint256_t(input.additive_constant) + 1;
    } else if (max != uint256_t(0)) {
        maximum_value = max;
    } else {
        maximum_value = DEFAULT_MAXIMUM_LIMB;
    }
}

The default value for the argument max is zero, and if max is zero, then maximum_value is set to DEFAULT_MAXIMUM_LIMB, which is a different value.

Thus, a caller that passes zero for max as the actual intended maximum value will have this choice overriden, with maximum_value set to the much larger DEFAULT_MAXIMUM_LIMB instead. This kind of call actually appears in the codebase, for example in the bigfield<Builder, T>::self_reduce() function in cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp:

// TODO: implicit assumption here - NUM_LIMB_BITS large enough for all the quotient
quotient.binary_basis_limbs[0] = Limb(quotient_limb, uint256_t(1) << maximum_quotient_bits);
quotient.binary_basis_limbs[1] = Limb(field_t<Builder>::from_witness_index(context, context->zero_idx), 0);
quotient.binary_basis_limbs[2] = Limb(field_t<Builder>::from_witness_index(context, context->zero_idx), 0);
quotient.binary_basis_limbs[3] = Limb(field_t<Builder>::from_witness_index(context, context->zero_idx), 0);

Additionally, the max argument is ignored if input is a constant. In that case, it would be expected behavior that the function asserts that the constant satisfies the passed maximum and either uses the actual constant value as maximum_value or the passed max. Actual behavior of the constructor is not documented.

Impact

The unnecessarily large maximum_value for limbs that the caller has ensured are zero can cause unnecessary reductions on later usage.

Should a caller pass a calculated constant to this constructor along with an incorrectly estimated maximum, expecting the constructor to check this bound, unintended behavior can arise, depending on the caller code.

Recommendations

Change the default value of max to DEFAULT_MAXIMUM_LIMB, and then do not treat any special value of max different than the others. This will ensure that a caller passing a value for max will obtain the intended behavior.

Add an assert to check that input.additive_constant <= max in the case of a constant input.

Remediation