add fuzz tests about field operations. #1385

1. 
   YafeiXie1 commented at 8:08 am on July 27, 2023: none

   Thanks for the previous review. This PR:

   1. changes on the function names in the scalar operations tests.
   2. remove the randomness in fuzz_scalar_shift test, the value is computed from size.
   3. add the function that decide the test based on an environment variable.
   4. add fuzz tests about field operations. Could you give me some suggestions?
2. add fuzz tests about field operations. 35be903a82
3. real-or-random added the label assurance on Jul 27, 2023
4. in src/fuzz.c:598 in 35be903a82 outdated

   593+        selected_fuzz_function = &fuzz_field_cmov;
   594+    } else if (strcmp(test_name, "field_storage_cmov") == 0) {
   595+        selected_fuzz_function = &fuzz_field_storage_cmov;
   596+    } else {
   597+        fprintf(stderr, "Unknown fuzz test selected using FUZZ environment variable: %s\n", test_name);
   598+        assert(false);
   

   ---

   ---

   

   sipa commented at 7:35 pm on August 1, 2023:

   false doesn’t exist as a keyword in C89 (which the libsecp256k1 source code tries to use with the least amount of extensions possible). You can use assert(0);.
5. in src/fuzz.c:467 in 35be903a82 outdated

   462+        secp256k1_fe_set_b32_mod(&a, data);
   463+        secp256k1_fe_inv(&r1, &a);
   464+        if (secp256k1_fe_normalizes_to_zero(&a)) {
   465+            CHECK(secp256k1_fe_normalizes_to_zero(&r1));
   466+        }
   467+        else {
   

   ---

   ---

   

   sipa commented at 7:36 pm on August 1, 2023:

   Coding style nit: } else { can go on one line.
6. in src/fuzz.c:130 in 35be903a82 outdated

   125+        secp256k1_scalar_set_b32(&a, data, NULL);
   126+        secp256k1_scalar_inverse(&r1, &a);
   127+        if (secp256k1_scalar_is_zero(&a)) {
   128+            CHECK(secp256k1_scalar_is_zero(&r1));
   129+        }
   130+        else {
   

   ---

   ---

   

   sipa commented at 8:23 pm on August 1, 2023:

   Style nit: } else { on one line.
7. in src/fuzz.c:146 in 35be903a82 outdated

   141+        secp256k1_scalar_set_b32(&a, data, NULL);
   142+        secp256k1_scalar_inverse_var(&r1, &a);
   143+        if (secp256k1_scalar_is_zero(&a)) {
   144+            CHECK(secp256k1_scalar_is_zero(&r1));
   145+        }
   146+        else {
   

   ---

   ---

   

   sipa commented at 8:23 pm on August 1, 2023:

   Style nit: } else { on one line.
8. in src/fuzz.c:170 in 35be903a82 outdated

   165+static void fuzz_scalar_shift(const uint8_t *data, size_t size) {
   166+    if (size > 31) {
   167+        int bit, r1, r2;     
   168+        secp256k1_scalar a;
   169+        secp256k1_scalar_set_b32(&a, data, NULL);
   170+        bit = 1 + (size % 15);
   

   ---

   ---

   

   sipa commented at 8:25 pm on August 1, 2023:

   I don’t recommend using the fuzzer input’s length as input itself; I don’t think it’s really designed to covney much information.

   Suggestion: bit = 1 + (data[32] % 15), and if (size >= 33) {.

9. in src/fuzz.c:12 in 35be903a82 outdated

    7+#include "secp256k1.c"
    8+
    9+/*** Scalar Operation ***/
   10+/* Test commutativity of scalar addition */ 
   11+static void fuzz_scalar_add_commutativty(const uint8_t *data, size_t size) {
   12+    if (size > 63) {        
   

   ---

   ---

   

   sipa commented at 8:25 pm on August 1, 2023:

   Tiny nit, but I think it’s a bit more readable to use if (size >= 64) { (here and elsewhere), as the 64 is the actual size we need.
10. in src/fuzz.c:212 in 35be903a82 outdated

    207+/*** Field Operation ***/
    208+/* Test the field element equality and comparison operations. */
    209+static void fuzz_field_equality(const uint8_t *data, size_t size) {
    210+    if (size > 31) {
    211+        secp256k1_fe fe;    
    212+        secp256k1_fe_set_b32_mod(&fe, data);
    

    ---

    ---

    

    sipa commented at 8:35 pm on August 1, 2023:

    This will only construct very “benign” field elements, which are perhaps not the most likely ones to trigger issues.

    Going beyond this is non-trivial though. You’ll probably want to write a helper function to construct a field element with specified representation directly (so the fe.d[0..4] elements, in particular) and then bail out if it turns out to be invalid. Then you’ll need to set magnitude/normalization appropriately as well.

    This will involve a number of changes, and you’ll want to read up on the field element representation (see field_5x52.h, let’s ignore the 26-bit field for now). It’ll also mean some refactoring of secp256k1_fe_impl_verify.

11. Merge branch 'bitcoin-core:master' into fuzz d59c63a904
12. 
    sipa commented at 2:24 pm on August 21, 2023: contributor

    Superseded by #1407
13. sipa closed this on Aug 21, 2023

    ---

14. YafeiXie1 deleted the branch on Aug 29, 2023

Contributors
YafeiXie1 sipa

Labels
assurance