The fix to https://cantina.xyz/code/c486d600-bed0-4fc6-aed1-de759fd29fa2/findings/21 has a typo that still results in the highest limb of pc being range checked to 8-bits instead of 6-bits.
In the AIR, we do
|
for (i, limb) in pc_limbs.iter().skip(1).enumerate() { |
for (i, limb) in pc_limbs.iter().skip(1).enumerate() {
if i == pc_limbs.len() - 1 {
It should be
for (i, limb) in pc_limbs.iter().enumerate().skip(1) {
Right now the if statement is never triggered because the enumeration gives i=0,1,2 when we instead want i=1,2,3. What this means is that pc_limbs[3] is range checked to 8-bits instead of 6-bits.
This leads to a vulnerability where the pc_limbs decomposition differs from the true pc, which means a malicious prover can make the destination register take a different value than the AUIPC instruction dictates, by making the decomposition overflow the BabyBear field.
jonathanpwang Finder
The fix to https://cantina.xyz/code/c486d600-bed0-4fc6-aed1-de759fd29fa2/findings/21 has a typo that still results in the highest limb of
pcbeing range checked to 8-bits instead of 6-bits.In the AIR, we do
openvm/extensions/rv32im/circuit/src/auipc/core.rs
Line 135 in 0f94c8a
It should be
Right now the if statement is never triggered because the enumeration gives
i=0,1,2when we instead wanti=1,2,3. What this means is thatpc_limbs[3]is range checked to 8-bits instead of 6-bits.This leads to a vulnerability where the
pc_limbsdecomposition differs from the truepc, which means a malicious prover can make the destination register take a different value than the AUIPC instruction dictates, by making the decomposition overflow the BabyBear field.