Wrong verifying key contract permutation length can be considered valid by validateVKPermutationsLength

[obatirou]

The function validateVKPermutationsLength is used to validate the number of permutations in the verifying key contract corresponds to the circuit used to generate proof for the number of cryptocurrencies the custodian committed to. The Summa contract assumes that permutations commitments begins at bytes 0x2e0 in the vKContract (source)
This can lead to wrongfully consider valid vkContracts with different permutation commitments from the circuit used by the custodian. A simple case would be a verifying contract with 2 fixed commitments but one permutation commitment less than the one being used. In this case, all others things being equals, the permutations commitments would not begin at bytes 0x2e0 but at 0x0320. As the length of the bytes of the contracts are still equals due to one permutation in less, validateVKPermutationsLength would consider this vkcontract valid but in fact is not.

[sebastiantf]

I suppose this is kinda analogous / related to how SnarkVerifier loads exactly 0x0560 bytes from the vk contract

summa-solvency/contracts/src/SnarkVerifier.sol

Line 218 in fec83a7

extcodecopy(vk, VK_MPTR, 0x00, 0x0560)

If the no. of commitments change, then the length of the bytecode loaded also changes, no? But I suppose extcodesize could be used while sacrificing some gas costs for this.

But I think the point I'm tryna make is maybe Summa.sol is justified (borrows the behavior in SnarkVerifier.sol) in using a hardcoded length in lieu of SnarkVerifier using hardcoded ptr location / lengths. But the latter the generated whereas the former is written. So I suppose there should some manual/automated verification that this pointer is correct on each iteration?

Although I did not notice any of the verifiers using pointers starting from the commitments. They all seem to only use the ptrs until neg_s_g2_y_2:

summa-solvency/contracts/src/VerifyingKey.sol

Line 28 in fec83a7

mstore(0x0280, 0x2b1cbb3e521edf5a622d82762a44a5e63f1e50b332d71154a4a7958d6011deff) // neg_s_g2_y_2

[0xalizk]

I don't understand this part:

A simple case would be a verifying contract with 2 fixed commitments but one permutation commitment less than the one being used
Could you elaborate?

[obatirou]

I don't understand this part:

A simple case would be a verifying contract with 2 fixed commitments but one permutation commitment less than the one being used
Could you elaborate?

Sure, it is a follow up of this discussion https://discord.com/channels/877252171983360072/1230201530582568962/1230453113199263777
Commitments are hardcoded in the Halo2VerifyingKey

summa-solvency/contracts/src/VerifyingKey.sol

Line 30 in fec83a7

mstore(0x02c0, 0x2c1e30781f735e7977820d4ca9145010e28dcc808ee0f1e976477289057b7aec) // fixed_comms[0].y

Hence, if a different VerifyingKey contract is created with same bytecode size but different commitments, it can be considered valid by validateVKPermutationsLength function

summa-solvency/contracts/src/Summa.sol

Line 151 in fec83a7

uint256 startOffsetForPermutations = 0x2e0; // The value can be observed in the VerificationKey contract, the offset is pointing after all the parameters and the fixed column commitment

In the current one, https://github.com/zBlock-2/summa-solvency/blob/fec83a747ead213261aecfaf4a01b43fff9731ee/contracts/src/VerifyingKey.sol there are 1 fixed commitments and 10 permutation commitments.
We could imagine a new contracts with 2 fixed commitments and 9 permutations commitments, this one would still be considered valid by validateVKPermutationsLength function even if the number of permutations commitments is invalid.

[sifnoc]

Although I did not notice any of the verifiers using pointers starting from the commitments. They all seem to only use the ptrs until neg_s_g2_y_2:

The permutation commitments are utilized during the verification process in SnarkVerifier.
This verifier copies all verifying keys as shown here:

summa-solvency/contracts/src/SnarkVerifier.sol

Line 218 in fec83a7

extcodecopy(vk, VK_MPTR, 0x00, 0x0560)

[sifnoc]

To further clarify the example, let's consider how manipulation could occur in the current Summa implementation.
A prover wishing to manipulate the validateVKPermutationsLength function might modify the circuit to accept additional public inputs.
Although this scenario does not perfectly align with the case discussed, as the increase in fixed commitments would change the base number of permutations from 2 to 3 in the formula, resulting in a modified formula: numPermutations = 3 + (balanceByteRange/2) * numberOfCurrencies instead of

summa-solvency/contracts/src/Summa.sol

Line 147 in fec83a7

uint256 numPermutations = 2 +

This demonstrates that adjustments can be made by altering the circuit configuration.

Subsequently, the prover would need to send the commitment transaction with more public inputs. Here, I assume the Summa contract has been modified to accommodate two public inputs by adjusting the arguments here:

summa-solvency/contracts/src/Summa.sol

Line 239 in fec83a7

uint[] memory args = new uint[](1);

It's important to note that the prover would need to modify other hardcoded numbers to achieve a valid result from the validateVKPermutationsLength by altering the number of permutations, without changing the hardcoded startOffsetForPermutations. More importantly, users can easily notice a commitment transaction designed for two fixed columns, which is inconsistent with having just one public input.

Furthermore, another variable, offsetAfterLastPermutation, depends on numPermutations as well as startOffsetForPermutations.

summa-solvency/contracts/src/Summa.sol

Lines 154 to 156 in fec83a7

	uint256 offsetAfterLastPermutation = startOffsetForPermutations +
	numPermutations *
	0x40;

It can also produce a valid result with an incorrect number of currencies and a different balanceByteRange.
For example:

• With a balanceByteRange of 4 and numberOfCurrencies as 5, it calculates numPermutations as 20.
• The same result occurs if numberOfCurrencies is 4 and balanceByteRange is 5.

This outcome follows the formula:

summa-solvency/contracts/src/Summa.sol

Lines 147 to 149 in fec83a7

	uint256 numPermutations = 2 +
	(balanceByteRange / 2) *
	numberOfCurrencies;

However, such discrepancies are typically obvious to users when they attempt to verify their proofs.