Description

This finding concerns the following constructor for bigfield:

template <typename Builder, typename T>
bigfield<Builder, T>::bigfield(const field_t<Builder>& low_bits_in,
                               const field_t<Builder>& high_bits_in,
                               const bool can_overflow,
                               const size_t maximum_bitlength)

After the if check on HasPlookup<Builder>, in the else branches, the method decompose_into_base4_accumulators of the class StandardCircuitBuilder_<FF> is called. This function takes raw witness indexes as input, but the arguments are not normalized in the bigfield constructor beforehand, as can be seen here in the case of the lower bits:

low_accumulator = context->decompose_into_base4_accumulators(
    low_bits_in.witness_index, static_cast<size_t>(NUM_LIMB_BITS * 2), "bigfield: low_bits_in too large.");

This applies also for the higher bits.

Impact

If the additive or multiplicative constant of the field_t values low_bits_in or high_bits_in are different from 1 and 0, respectively, the constructor will construct a bigfield element with incorrect values.

Recommendations

Normalize the witness before passing the witness index as an argument to decompose_into_base4_accumulators, for example in the lower-bits case,

low_accumulator = context->decompose_into_base4_accumulators(
-   low_bits_in.witness_index, static_cast<size_t>(NUM_LIMB_BITS * 2), "bigfield: low_bits_in too large.");
+   low_bits_in.normalize().witness_index, static_cast<size_t>(NUM_LIMB_BITS * 2), "bigfield: low_bits_in too large.");

and similarly for the higher-bits case.

Remediation