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

Add tests for fuzzing on scalar operation #1377

Closed
wants to merge 2 commits into from

Conversation

YafeiXie1
Copy link

Hello, I’m trying to develop the fuzz test using libfuzzer. This PR shows the tests about several properties of scalar operations. Could you give me some suggestions?

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need time.h.

#include <string.h>
#include <time.h>

#include "scalar_impl.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of including all these individual files, you can just include secp256k1.c (the main implementation file includes all the necessary code from all modules).

When you do that, you'll also need the precomputed tables (which are in separate .c files) when compiling, as the ecmult logic needs them (even when you're not actually testing those yet).

For example, you'd build with

clang++ -g -O2 -fsanitize=undefined,address,fuzzer fuzz.c precomputed_ecmult.c precomputed_ecmult_gen.c -o fuzz

(this is just temporary before we integrate it into the build system)

}


/** Entry point of Libfuzzer **/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is some code to decide the test based on an environment variable:

typedef void (*fuzz_function)(const uint8_t* data, size_t size);

static fuzz_function selected_fuzz_function = NULL;

int LLVMFuzzerInitialize(int *argc, char ***argv) {
    const char* test_name = getenv("FUZZ");
    if (!test_name) {
        fprintf(stderr, "Select a fuzz test using the FUZZ environment variable\n");
        assert(false);
    }
    if (strcmp(test_name, "scalar_inverse") == 0) {
        selected_fuzz_test = &fuzz_scalar_inverse;
    } else if (strcmp(test_name, "scalar_negate") == 0) {
        selected_fuzz_test = &fuzz_scalar_negate;
    } else if ...
        ...
    } else {
        fprintf(stderr, "Unknown fuzz test selected using FUZZ environment variable: %s\n", test_name);
        assert(false);
    }
    return 0;
}

int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
    selected_fuzz_test(data, size);
    return 0;
}

(I haven't tried to compile/run the above, there may be bugs)

You'd then invoke the tests using

FUZZ=scalar_inverse ./fuzz

/*** Scalar Operation ***/

/* Test commutativity of scalar addition */
static void fuzz_commutate_add(const uint8_t *data, size_t size) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (= unimportant comment, feel free to ignore if you disagree): I find "cummutate" a bit unnatural, what about fuzz_add_commutativity for example?

}

/* Test associativity of scalar addition */
static void fuzz_associate_add(const uint8_t *data, size_t size) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: fuzz_add_associativity ?



/* Test scalar multiplication with zero */
static void fuzz_zero_mul(const uint8_t *data, size_t size) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: fuzz_scalar_mul_zero ?

int bit, r1, r2;
secp256k1_scalar a;
secp256k1_scalar_set_b32(&a, data, NULL);
bit = 1 + secp256k1_testrand_int(15);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot use randomness in fuzz tests, because the fuzzer cannot control the randomness (coverage will be unrelated and nondeterministic, which will confuse the fuzzer's tracking of which seeds are useful). You should get the value from the fuzzer data/size here too.

#include "ecmult_gen_impl.h"
#include "int128_impl.h"
#include "scratch_impl.h"
#include "testrand_impl.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't include this; you should never need randomness in fuzz tests.

}

/* Test r1+r2*lambda = a */
static void fuzz_scalar_splite_lambda(const uint8_t *data, size_t size) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: splite -> split

}
**/


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a newline at the end of the file.

@YafeiXie1 YafeiXie1 closed this Jul 27, 2023
@YafeiXie1 YafeiXie1 deleted the fuzz_test branch July 27, 2023 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants