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

Added fuzzer #255

Merged
merged 2 commits into from
Feb 2, 2024
Merged

Added fuzzer #255

merged 2 commits into from
Feb 2, 2024

Conversation

povilassl
Copy link
Contributor

No description provided.

@sauliusgrigaitis
Copy link
Member

@povilassl I tried to mess the code of arkworks backend and confirmed that I involved the bug by running cargo test in arkworks directory. However, when I run fuzzer for arkworks the fuzzer doesn't tell that it got differences.

Please show that the fuzzer actually works.

@sauliusgrigaitis
Copy link
Member

Still doesn't work for me.

Steps to reproduce:

Checkout this branch.

Introduce some but, something like this:

diff --git a/arkworks/src/kzg_types.rs b/arkworks/src/kzg_types.rs
index 57e499e..90a3453 100644
--- a/arkworks/src/kzg_types.rs
+++ b/arkworks/src/kzg_types.rs
@@ -40,7 +40,7 @@ fn bytes_be_to_uint64(inp: &[u8]) -> u64 {
 
 const BLS12_381_MOD_256: [u64; 4] = [
     0xffffffff00000001,
-    0x53bda402fffe5bfe,
+    0x53bda402fffe5bf1,
     0x3339d80809a1d805,
     0x73eda753299d7d48,
 ];
diff --git a/run-c-kzg-4844-fuzz.sh b/run-c-kzg-4844-fuzz.sh
index eab754a..681328b 100644
--- a/run-c-kzg-4844-fuzz.sh
+++ b/run-c-kzg-4844-fuzz.sh
@@ -11,10 +11,10 @@ print_msg () {
 
 ###################### parallel & backend configuration ######################
 
-parallel=false
-use_arkmsm=false
+parallel=true
+use_arkmsm=true
 use_bgmw=false
-backend="unknown"
+backend="arkworks"
 
 while [[ -n $# ]]; do
   case $1 in

Tests doesn't pass. However the fuzzer don't find anything.

@ArtiomTr
Copy link
Contributor

I am not familiar with the arkworks code, and I don't know what the BLS12_381_MOD_256 constant does, but I've tried to add very simple change, inside eip_4844 C bindings:

diff --git a/arkworks/src/eip_4844.rs b/arkworks/src/eip_4844.rs
index 6f1ab5e..fb78e9f 100644
--- a/arkworks/src/eip_4844.rs
+++ b/arkworks/src/eip_4844.rs
@@ -9,7 +9,7 @@ use kzg::eip_4844::{
     load_trusted_setup_rust, verify_blob_kzg_proof_batch_rust, verify_blob_kzg_proof_rust,
     verify_kzg_proof_rust, Blob, Bytes32, Bytes48, CKZGSettings, KZGCommitment, KZGProof,
     BYTES_PER_FIELD_ELEMENT, BYTES_PER_G1, BYTES_PER_G2, C_KZG_RET, C_KZG_RET_BADARGS,
-    C_KZG_RET_OK, FIELD_ELEMENTS_PER_BLOB, TRUSTED_SETUP_NUM_G1_POINTS,
+    C_KZG_RET_ERROR, C_KZG_RET_OK, FIELD_ELEMENTS_PER_BLOB, TRUSTED_SETUP_NUM_G1_POINTS,
     TRUSTED_SETUP_NUM_G2_POINTS,
 };
 use kzg::{cfg_into_iter, Fr, G1};
@@ -153,6 +153,8 @@ pub unsafe extern "C" fn blob_to_kzg_commitment(
     blob: *const Blob,
     s: &CKZGSettings,
 ) -> C_KZG_RET {
+    return C_KZG_RET_ERROR;
+
     if TRUSTED_SETUP_NUM_G1_POINTS == 0 {
         // FIXME: load_trusted_setup should set this value, but if not, it fails
         TRUSTED_SETUP_NUM_G1_POINTS = FIELD_ELEMENTS_PER_BLOB

Running fuzzer fails quickly, with error:

fuzz: elapsed: 0s, gathering baseline coverage: 0/256 completed
fuzz: elapsed: 3s, gathering baseline coverage: 0/256 completed
fuzz: elapsed: 5s, gathering baseline coverage: 1/256 completed
--- FAIL: FuzzBlobToKZGCommitment (5.10s)
    --- FAIL: FuzzBlobToKZGCommitment (0.15s)
        main_test.go:45: 
                Error Trace:    /home/sirse/rust-kzg/arkworks/c-kzg-4844/bindings/go/kzg-fuzz/main_test.go:45
                                                        /home/sirse/rust-kzg/arkworks/c-kzg-4844/bindings/go/kzg-fuzz/value.go:596
                                                        /home/sirse/rust-kzg/arkworks/c-kzg-4844/bindings/go/kzg-fuzz/value.go:380
                                                        /home/sirse/rust-kzg/arkworks/c-kzg-4844/bindings/go/kzg-fuzz/fuzz.go:335
                Error:          Not equal: 

Also, I've tried to mess arkworks code, by adding one generator to blob_to_kzg_commitment_rust function result:

diff --git a/kzg/src/eip_4844.rs b/kzg/src/eip_4844.rs
index 2600949..82389d5 100644
--- a/kzg/src/eip_4844.rs
+++ b/kzg/src/eip_4844.rs
@@ -236,7 +236,7 @@ pub fn blob_to_kzg_commitment_rust<
 ) -> Result<TG1, String> {
     let polynomial = blob_to_polynomial(blob)?;
 
-    Ok(poly_to_kzg_commitment(&polynomial, settings))
+    Ok(poly_to_kzg_commitment(&polynomial, settings).add(&TG1::generator()))
 }
 
 pub fn compute_powers<TFr: Fr>(base: &TFr, num_powers: usize) -> Vec<TFr> {

And fuzzer failed again, with error:

fuzz: elapsed: 0s, gathering baseline coverage: 0/266 completed
fuzz: elapsed: 3s, gathering baseline coverage: 0/266 completed
fuzz: elapsed: 6s, gathering baseline coverage: 1/266 completed
--- FAIL: FuzzBlobToKZGCommitment (5.51s)
    --- FAIL: FuzzBlobToKZGCommitment (0.38s)
        main_test.go:47: 
                Error Trace:    /home/sirse/rust-kzg/arkworks/c-kzg-4844/bindings/go/kzg-fuzz/main_test.go:47
                                                        /home/sirse/rust-kzg/arkworks/c-kzg-4844/bindings/go/kzg-fuzz/value.go:596
                                                        /home/sirse/rust-kzg/arkworks/c-kzg-4844/bindings/go/kzg-fuzz/value.go:380
                                                        /home/sirse/rust-kzg/arkworks/c-kzg-4844/bindings/go/kzg-fuzz/fuzz.go:335
                Error:          Not equal: 
                                expected: []byte{0x80, 0x6, 0xe6, 0x51, 0xe0, 0x74, 0xf5, 0x29, 0x3c, 0x77, 0xc, 0xec, 0xa, 0x47, 0xaf, 0xf9, 0xd, 0xe8, 0xc7, 0x5d, 0x82, 0x75, 0x9b, 0x17, 0xd4, 0x99, 0x7e, 0x80, 0x86, 0x60, 0x47, 0x5c, 0xad, 0x7d, 0xb9, 0xc4, 0xd2, 0x7c, 0x27, 0xc, 0xfe, 0xcf, 0xa2, 0xfc, 0xab, 0xd4, 0xae, 0x8d}
                                actual  : []byte{0xaa, 0x2b, 0xcf, 0x5f, 0x97, 0x28, 0x31, 0xcc, 0xfd, 0x4d, 0x5a, 0xbd, 0xa8, 0x98, 0x93, 0x29, 0xec, 0xb8, 0xce, 0x57, 0x3b, 0xf5, 0xa9, 0xc7, 0x25, 0x51, 0x7f, 0xae, 0xb3, 0x97, 0x1f, 0xcf, 0x50, 0xc6, 0x12, 0x83, 0x38, 0xb0, 0x5, 0x91, 0xae, 0x5, 0x7b, 0x1c, 0x24, 0x83, 0x36, 0x32}
                            
                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1,5 +1,5 @@
                                 ([]uint8) (len=48) {
                                - 00000000  80 06 e6 51 e0 74 f5 29  3c 77 0c ec 0a 47 af f9  |...Q.t.)<w...G..|
                                - 00000010  0d e8 c7 5d 82 75 9b 17  d4 99 7e 80 86 60 47 5c  |...].u....~..`G\|
                                - 00000020  ad 7d b9 c4 d2 7c 27 0c  fe cf a2 fc ab d4 ae 8d  |.}...|'.........|
                                + 00000000  aa 2b cf 5f 97 28 31 cc  fd 4d 5a bd a8 98 93 29  |.+._.(1..MZ....)|
                                + 00000010  ec b8 ce 57 3b f5 a9 c7  25 51 7f ae b3 97 1f cf  |...W;...%Q......|
                                + 00000020  50 c6 12 83 38 b0 05 91  ae 05 7b 1c 24 83 36 32  |P...8.....{.$.62|
                                 }
                Test:           FuzzBlobToKZGCommitment
    
    Failing input written to testdata/fuzz/FuzzBlobToKZGCommitment/005c380a7af47341
    To re-run:
    go test -run=FuzzBlobToKZGCommitment/005c380a7af47341
FAIL
exit status 1
FAIL    fuzz    7.468s

@sauliusgrigaitis sauliusgrigaitis merged commit 1c9f443 into grandinetech:main Feb 2, 2024
24 checks passed
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