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.

Sign up for GitHub

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

Jump to bottom

Add Reproducible Error Injection (rebased) #21668

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Add Reproducible Error Injection (rebased) #21668

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bee2d87
Add Reproducible Error Injection
bernd-edlinger May 20, 2022
9c489bf
fixup! Add Reproducible Error Injection
bernd-edlinger Aug 7, 2023
190ce20
fixup! Add Reproducible Error Injection
bernd-edlinger Aug 8, 2023
52f5b55
fixup! Add Reproducible Error Injection
bernd-edlinger Aug 11, 2023
930d2bd
fixup! Add Reproducible Error Injection
bernd-edlinger Aug 11, 2023
539b6a5
fixup! Add Reproducible Error Injection
bernd-edlinger Aug 12, 2023
a87f07c
fixup! Add Reproducible Error Injection
bernd-edlinger Aug 12, 2023
453914f
fixup! Add Reproducible Error Injection
bernd-edlinger Aug 12, 2023
f5a4fa4
fixup! Add Reproducible Error Injection
bernd-edlinger Aug 18, 2023
154463b
fixup! Add Reproducible Error Injection
bernd-edlinger Aug 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 25 additions & 0 deletions .github/workflows/fuzz-testrun.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
name: Error Injection Testrun

on:
pull_request:
push:
schedule:
- cron: '0 10 * * *'

permissions:
contents: read

jobs:
fuzzing_testrun:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: checkout fuzz/corpora submodule
run: git submodule update --init --depth 1 fuzz/corpora
- name: config
run: ./config --strict-warnings enable-asan enable-ubsan enable-rc5 enable-md2 enable-ec_nistp_64_gcc_128 -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -DERROR_INJECT -DERROR_CALLSTACK && perl configdata.pm --dump
- name: make
run: make -s -j4
- name: fuzzing...
run: ./util/shlib_wrap.sh ./apps/openssl version -a && cd fuzz && (sh -c 'sleep 3600; touch stop.signal; sleep 60; test -f stop.signal && killall -6 -r ".*-test";' &) && ASAN_OPTIONS=handle_abort=true ./testrun.sh && test ! -f *-test.out
3 changes: 3 additions & 0 deletions fuzz/asn1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ int FuzzerInitialize(int *argc, char ***argv)
{
FuzzerSetRand();
pctx = ASN1_PCTX_new();
if (pctx == NULL)
return 0;

ASN1_PCTX_set_flags(pctx, ASN1_PCTX_FLAGS_SHOW_ABSENT |
ASN1_PCTX_FLAGS_SHOW_SEQUENCE | ASN1_PCTX_FLAGS_SHOW_SSOF |
ASN1_PCTX_FLAGS_SHOW_TYPE | ASN1_PCTX_FLAGS_SHOW_FIELD_STRUCT_NAME);
Expand Down
19 changes: 14 additions & 5 deletions fuzz/bignum.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
b4 = BN_new();
b5 = BN_new();
ctx = BN_CTX_new();
if (b1 == NULL || b2 == NULL || b3 == NULL
|| b4 == NULL || b5 == NULL || ctx == NULL)
goto err;

/* Divide the input into three parts, using the values of the first two
* bytes to choose lengths, which generate b1, b2 and b3. Use three bits
Expand All @@ -62,10 +65,13 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
s3 = buf[0] & 4;
++buf;
}
OPENSSL_assert(BN_bin2bn(buf, l1, b1) == b1);
Copy link
Member

Choose a reason for hiding this comment

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

Except in the case of a malloc failure this really should succeed. So by removing the assert you remove the possibility that the normal fuzzer (i.e. where we're not injecting malloc errors) could find an input that produces an unexpected failure. After this change if it has an input that produces an unexpected failure, the fuzzer will just ignore it.

We should really only avoid this assert in the case that ERROR_INJECT is defined.

The same comment applies to many of the other places where you are removing asserts.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I see what you mean, that's valid point.
Although I doubt that BN_bin2bn has any other failure mode than out-of-memory,
the other BN_mod_exp below might be a different story.
I made them go to the assertion and guard that in ifndef ERROR_INJECT as suggested in several places.

if (BN_bin2bn(buf, l1, b1) != b1)
goto err;
BN_set_negative(b1, s1);
OPENSSL_assert(BN_bin2bn(buf + l1, l2, b2) == b2);
OPENSSL_assert(BN_bin2bn(buf + l1 + l2, l3, b3) == b3);
if (BN_bin2bn(buf + l1, l2, b2) != b2)
goto err;
if (BN_bin2bn(buf + l1 + l2, l3, b3) != b3)
goto err;
BN_set_negative(b3, s3);

/* mod 0 is undefined */
Expand All @@ -74,8 +80,10 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
goto done;
}

OPENSSL_assert(BN_mod_exp(b4, b1, b2, b3, ctx));
OPENSSL_assert(BN_mod_exp_simple(b5, b1, b2, b3, ctx));
if (!BN_mod_exp(b4, b1, b2, b3, ctx))
goto err;
if (!BN_mod_exp_simple(b5, b1, b2, b3, ctx))
goto err;

success = BN_cmp(b4, b5) == 0;
if (!success) {
Expand All @@ -93,6 +101,7 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)

done:
t8m marked this conversation as resolved.
Show resolved Hide resolved
OPENSSL_assert(success);
err:
BN_free(b1);
BN_free(b2);
BN_free(b3);
Expand Down
58 changes: 33 additions & 25 deletions fuzz/bndiv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,8 @@
/* 256 kB */
#define MAX_LEN (256 * 1000)

static BN_CTX *ctx;
static BIGNUM *b1;
static BIGNUM *b2;
static BIGNUM *b3;
static BIGNUM *b4;
static BIGNUM *b5;

int FuzzerInitialize(int *argc, char ***argv)
{
b1 = BN_new();
b2 = BN_new();
b3 = BN_new();
b4 = BN_new();
b5 = BN_new();
ctx = BN_CTX_new();

OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS, NULL);
ERR_clear_error();

Expand All @@ -49,6 +35,22 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
size_t l1 = 0, l2 = 0;
/* s1 and s2 will be the signs for b1 and b2. */
int s1 = 0, s2 = 0;
BN_CTX *ctx;
BIGNUM *b1;
BIGNUM *b2;
BIGNUM *b3;
BIGNUM *b4;
BIGNUM *b5;

b1 = BN_new();
b2 = BN_new();
b3 = BN_new();
b4 = BN_new();
b5 = BN_new();
ctx = BN_CTX_new();
if (b1 == NULL || b2 == NULL || b3 == NULL
|| b4 == NULL || b5 == NULL || ctx == NULL)
goto err;

/* limit the size of the input to avoid timeout */
if (len > MAX_LEN)
Expand All @@ -69,9 +71,11 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
++buf;
l2 = len - l1;
}
OPENSSL_assert(BN_bin2bn(buf, l1, b1) == b1);
if (BN_bin2bn(buf, l1, b1) != b1)
goto err;
BN_set_negative(b1, s1);
OPENSSL_assert(BN_bin2bn(buf + l1, l2, b2) == b2);
if (BN_bin2bn(buf + l1, l2, b2) != b2)
goto err;
BN_set_negative(b2, s2);

/* divide by 0 is an error */
Expand All @@ -80,7 +84,12 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
goto done;
}

OPENSSL_assert(BN_div(b3, b4, b1, b2, ctx));
if (!BN_div(b3, b4, b1, b2, ctx))
goto err;
if (!BN_mul(b5, b3, b2, ctx))
goto err;
if (!BN_add(b5, b5, b4))
goto err;
if (BN_is_zero(b1))
success = BN_is_zero(b3) && BN_is_zero(b4);
else if (BN_is_negative(b1))
Expand All @@ -89,8 +98,6 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
else
success = (BN_is_negative(b3) == BN_is_negative(b2) || BN_is_zero(b3))
&& (!BN_is_negative(b4) || BN_is_zero(b4));
OPENSSL_assert(BN_mul(b5, b3, b2, ctx));
OPENSSL_assert(BN_add(b5, b5, b4));

success = success && BN_cmp(b5, b1) == 0;
if (!success) {
Expand All @@ -115,17 +122,18 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)

done:
OPENSSL_assert(success);
err:
BN_free(b1);
BN_free(b2);
BN_free(b3);
BN_free(b4);
BN_free(b5);
BN_CTX_free(ctx);
ERR_clear_error();

return 0;
}

void FuzzerCleanup(void)
{
BN_free(b1);
BN_free(b2);
BN_free(b3);
BN_free(b4);
BN_free(b5);
BN_CTX_free(ctx);
}
6 changes: 4 additions & 2 deletions fuzz/client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
if (client == NULL)
goto end;
OPENSSL_assert(SSL_set_min_proto_version(client, 0) == 1);
OPENSSL_assert(SSL_set_cipher_list(client, "ALL:eNULL:@SECLEVEL=0") == 1);
if (SSL_set_cipher_list(client, "ALL:eNULL:@SECLEVEL=0") != 1)
goto end;
SSL_set_tlsext_host_name(client, "localhost");
in = BIO_new(BIO_s_mem());
if (in == NULL)
Expand All @@ -84,7 +85,8 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
}
SSL_set_bio(client, in, out);
SSL_set_connect_state(client);
OPENSSL_assert((size_t)BIO_write(in, buf, len) == len);
if ((size_t)BIO_write(in, buf, len) != len)
goto end;
if (SSL_do_handshake(client) == 1) {
/* Keep reading application data until error or EOF. */
uint8_t tmp[1024];
Expand Down
5 changes: 4 additions & 1 deletion fuzz/cmp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,10 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
return 0;

in = BIO_new(BIO_s_mem());
OPENSSL_assert((size_t)BIO_write(in, buf, len) == len);
if ((size_t)BIO_write(in, buf, len) != len) {
BIO_free(in);
return 0;
}
msg = d2i_OSSL_CMP_MSG_bio(in, NULL);
if (msg != NULL) {
BIO *out = BIO_new(BIO_s_null());
Expand Down
5 changes: 4 additions & 1 deletion fuzz/cms.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
return 0;

in = BIO_new(BIO_s_mem());
OPENSSL_assert((size_t)BIO_write(in, buf, len) == len);
if ((size_t)BIO_write(in, buf, len) != len) {
BIO_free(in);
return 0;
}
cms = d2i_CMS_bio(in, NULL);
if (cms != NULL) {
BIO *out = BIO_new(BIO_s_null());
Expand Down
6 changes: 5 additions & 1 deletion fuzz/conf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)

conf = NCONF_new(NULL);
in = BIO_new(BIO_s_mem());
OPENSSL_assert((size_t)BIO_write(in, buf, len) == len);
if ((size_t)BIO_write(in, buf, len) != len) {
BIO_free(in);
NCONF_free(conf);
return 0;
}
NCONF_load_bio(conf, in, &eline);
NCONF_free(conf);
BIO_free(in);
Expand Down
6 changes: 3 additions & 3 deletions fuzz/decoder.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
EVP_PKEY_CTX *ctx = NULL;
BIO *bio;

bio = BIO_new(BIO_s_null());
dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, NULL, NULL, NULL, 0, NULL,
NULL);
if (dctx == NULL) {
if (dctx == NULL)
return 0;
}

bio = BIO_new(BIO_s_null());
if (OSSL_DECODER_from_data(dctx, &buf, &len)) {
EVP_PKEY *pkey2;

Expand Down
4 changes: 4 additions & 0 deletions fuzz/fuzz_rand.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,11 @@ void FuzzerSetRand(void)
if (!OSSL_PROVIDER_add_builtin(NULL, "fuzz-rand", fuzz_rand_provider_init)
|| !RAND_set_DRBG_type(NULL, "fuzz", NULL, NULL, NULL)
|| (r_prov = OSSL_PROVIDER_try_load(NULL, "fuzz-rand", 1)) == NULL)
#ifdef ERROR_INJECT
exit(0);
#else
exit(1);
#endif
}

void FuzzerClearRand(void)
Expand Down