From 08bf60db4e38b67e6def9552e7cbbc16ba67918a Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sat, 2 Jul 2022 14:39:50 +0200 Subject: [PATCH 1/5] rand: fix a possible division by zero ``` ../src/rand.c:627:33: runtime error: division by zero AddressSanitizer:DEADLYSIGNAL ================================================================= ==2080841==ERROR: AddressSanitizer: FPE on unknown address 0x00000040d3f7 (pc 0x00000040d3f7 bp 0x7ffc8f312330 sp 0x7ffc8f312250 T0) #0 0x40d3f7 in df_rand_dbus_objpath_string ../src/rand.c:627 #1 0x40fccb in test_df_rand_dbus_objpath_string ../test/test-rand.c:71 #2 0x7f5f57f803dd in g_test_run_suite_internal (/lib64/libglib-2.0.so.0+0x7e3dd) #3 0x7f5f57f80144 in g_test_run_suite_internal (/lib64/libglib-2.0.so.0+0x7e144) #4 0x7f5f57f808e1 in g_test_run_suite (/lib64/libglib-2.0.so.0+0x7e8e1) #5 0x7f5f57f8094c in g_test_run (/lib64/libglib-2.0.so.0+0x7e94c) #6 0x4103c8 in main ../test/test-rand.c:121 #7 0x7f5f576f854f in __libc_start_call_main (/lib64/libc.so.6+0x2954f) #8 0x7f5f576f8608 in __libc_start_main_alias_1 (/lib64/libc.so.6+0x29608) #9 0x403734 in _start (/home/fsumsal/repos/dfuzzer/build/test-rand+0x403734) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: FPE ../src/rand.c:627 in df_rand_dbus_objpath_string ==2080841==ABORTING ``` --- src/rand.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/rand.c b/src/rand.c index cd238a9..3d0c1ea 100644 --- a/src/rand.c +++ b/src/rand.c @@ -624,7 +624,10 @@ int df_rand_dbus_objpath_string(gchar **buf, guint64 iteration) * (each element needs at least two characters, hence size/2) and * we need at least one element (hence +-1). With that, generate * a pseudo-random number of elements in interval <1, size/2> */ - nelem = (rand() % (size / 2 - 1)) + 1; + g_assert(size >= 2); + /* Set the number of elements to 1 if size is < 4 to avoid dividing + * by zero */ + nelem = size < 4 ? 1 : (rand() % (size / 2 - 1)) + 1; ret = g_try_new(gchar, size + 1); if (!ret) From 37b7bb395fc6413c4795f07598dd12854cbde40a Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Wed, 29 Jun 2022 10:43:55 +0200 Subject: [PATCH 2/5] Introduce unit tests for df_rand_* stuff --- .github/workflows/run-tests.sh | 2 + meson.build | 26 +++++++ src/meson.build | 7 +- src/rand.c | 2 +- src/rand.h | 47 +------------ test/meson.build | 5 ++ test/test-rand.c | 122 +++++++++++++++++++++++++++++++++ 7 files changed, 163 insertions(+), 48 deletions(-) create mode 100644 test/meson.build create mode 100644 test/test-rand.c diff --git a/.github/workflows/run-tests.sh b/.github/workflows/run-tests.sh index 9e79332..1ae9517 100755 --- a/.github/workflows/run-tests.sh +++ b/.github/workflows/run-tests.sh @@ -2,6 +2,8 @@ set -ex +ninja -C ./build test + dfuzzer=("dfuzzer") if [[ "$TYPE" == valgrind ]]; then dfuzzer=("valgrind" "--leak-check=full" "--show-leak-kinds=definite" "--errors-for-leak-kinds=definite" "--error-exitcode=42" "dfuzzer") diff --git a/meson.build b/meson.build index e5b77d0..52b2684 100644 --- a/meson.build +++ b/meson.build @@ -7,6 +7,8 @@ project('dfuzzer', 'c', ], ) +tests = [] + libgio = dependency('gio-2.0', required : true) xsltproc = find_program('xsltproc', required: false) @@ -21,6 +23,7 @@ config_h = configure_file( add_project_arguments('-include', 'config.h', language : 'c') subdir('src') +subdir('test') executable( 'dfuzzer', @@ -66,3 +69,26 @@ if get_option('dfuzzer-test-server') endif install_data('src/dfuzzer.conf', install_dir : get_option('sysconfdir')) + +foreach tuple : tests + sources = tuple[0] + name = '@0@'.format(sources[0]).split('/')[-1].split('.')[0] + + exe = executable( + name, + dfuzzer_util_sources + sources, + include_directories : include_directories('src/'), + dependencies : [libgio], + ) + + # See: https://docs.gtk.org/glib/testing.html#using-meson + test(name, exe, + timeout : 60, + env : [ + 'G_TEST_SRCDIR=@0@'.format(meson.current_source_dir()), + 'G_TEST_BUILDDIR=@0@'.format(meson.current_build_dir()), + ] + ) +endforeach + +# vi: sw=8 ts=8 et: diff --git a/src/meson.build b/src/meson.build index e408247..645bafc 100644 --- a/src/meson.build +++ b/src/meson.build @@ -1,7 +1,6 @@ -dfuzzer_sources = files( +dfuzzer_util_sources = files( 'bus.c', 'bus.h', - 'dfuzzer.c', 'fuzz.c', 'fuzz.h', 'introspection.c', @@ -16,6 +15,10 @@ dfuzzer_sources = files( 'util.h', ) +dfuzzer_sources = dfuzzer_util_sources + files( + 'dfuzzer.c', +) + dfuzzer_test_server_sources = files( 'dfuzzer-test-server.c', ) diff --git a/src/rand.c b/src/rand.c index 3d0c1ea..281abd2 100644 --- a/src/rand.c +++ b/src/rand.c @@ -445,7 +445,7 @@ gdouble df_rand_gdouble(guint64 iteration) } } -static gunichar df_rand_unichar(guint16 *width) +gunichar df_rand_unichar(guint16 *width) { gunichar uc = 0; diff --git a/src/rand.h b/src/rand.h index 338052c..247f11d 100644 --- a/src/rand.h +++ b/src/rand.h @@ -87,54 +87,11 @@ guint64 df_rand_guint64(guint64 iteration); */ gdouble df_rand_gdouble(guint64 iteration); -/** - * @function Allocates memory for pseudo-random string of size counted - * by adding generated pseudo-random number from interval <0, CHAR_MAX> - * to df_str_len (this mechanism is responsible for generating bigger strings - * by every call of df_rand_string()). Then pseudo-random string is generated - * and stored in buf. At the beginning strings from global array df_str_def - * are used. Warning: buf should be freed outside this module by callee - * of this function. - * @param buf Address of pointer on buffer where generated string - * will be stored - * @return 0 on success, -1 on error - */ -int df_rand_string(gchar **buf, guint64 iteration); +gunichar df_rand_unichar(guint16 *width); -/** - * @function Allocates memory for pseudo-random object path string of size - * counted by adding 1 to size variable on every call of function to maximum - * size of MAX_OBJECT_PATH_LENGTH. On every call pseudo-random object path string is generated - * into buf buffer. - * Warning: buf should be freed outside this module by callee of this - * function. - * @param buf Address of pointer on buffer where generated object path string - * will be stored - * @return 0 on success, -1 on error - */ +int df_rand_string(gchar **buf, guint64 iteration); int df_rand_dbus_objpath_string(gchar **buf, guint64 iteration); - -/** - * @function Allocates memory for pseudo-random signature string of size - * counted by adding 1 to size variable on every call of function to maximum - * size of MAX_SIGNATURE_LENGTH. On every call pseudo-random signature string is generated - * by random access into global variable df_sig_def which contains all D-Bus - * signatures and copying signature into buf buffer. - * Warning: buf should be freed outside this module by callee of this - * function. - * @param buf Address of pointer on buffer where generated signature string - * will be stored - * @return 0 on success, -1 on error - */ int df_rand_dbus_signature_string(gchar **buf, guint64 iteration); - -/** - * @function Creates Gvariant containing pseudo-random string. At the beginning - * strings from global array df_str_def are used. - * @param var Address of pointer on GVariant where new Gvariant value - * will be stored - * @return 0 on success, -1 on error - */ int df_rand_GVariant(GVariant **var, guint64 iteration); /** diff --git a/test/meson.build b/test/meson.build new file mode 100644 index 0000000..9a3b05e --- /dev/null +++ b/test/meson.build @@ -0,0 +1,5 @@ +tests += [ + [files('test-rand.c')], +] + +# vi: sw=8 ts=8 et: diff --git a/test/test-rand.c b/test/test-rand.c new file mode 100644 index 0000000..3bbd4ec --- /dev/null +++ b/test/test-rand.c @@ -0,0 +1,122 @@ +#include +#include +#include + +#include "rand.h" +#include "util.h" + +#define RAND_TEST_ITERATIONS 5000 + +static void test_df_rand_unichar(void) +{ + guint16 width; + gunichar uc; + + /* Test if the function asserts on invalid width */ + if (g_test_subprocess()) { + /* This section runs in a subprocess */ + width = 5; + + (void) df_rand_unichar(&width); + } + + /* Respawn the current test in a subprocess */ + g_test_trap_subprocess (NULL, 0, 0); + /* The forked process above should have failed */ + g_test_trap_assert_failed(); + + /* Test explicit (and valid) 1 - 4 B wide characters */ + for (guint16 i = 1; i < 5; i++) { + uc = 0; + width = i; + + uc = df_rand_unichar(&width); + + /* The returned unichar should be in an UTF-8 range */ + g_assert_true(uc <= 0x10FFFF); + /* And the width should remain unchanged */ + g_assert_true(i == width); + } + + /* Test width == 0, which should give us a pseudo-random 1 - 4 B wide unichar */ + for (guint32 i = 0; i < RAND_TEST_ITERATIONS; i++) { + width = uc = 0; + + uc = df_rand_unichar(&width); + + /* The returned unichar should be in an UTF-8 range */ + g_assert_true(uc <= 0x10FFFF); + /* And the width should be in a valid range */ + g_assert_true(width > 0 && width < 5); + } +} + +static void test_df_rand_string(void) +{ + for (guint32 i = 0; i < RAND_TEST_ITERATIONS; i++) { + g_autoptr(gchar) str = NULL; + /* Test the "upper" guint64 interval in the second half of the iterations */ + guint64 iteration = i < RAND_TEST_ITERATIONS / 2 ? i : (guint64) g_test_rand_int_range(0, G_MAXINT32) + G_MAXINT32; + + g_assert_true(df_rand_string(&str, iteration) == 0); + g_assert_nonnull(str); + } +} + +static void test_df_rand_dbus_objpath_string(void) +{ + for (guint32 i = 0; i < RAND_TEST_ITERATIONS; i++) { + g_autoptr(gchar) str = NULL; + /* Test the "upper" guint64 interval in the second half of the iterations */ + guint64 iteration = i < RAND_TEST_ITERATIONS / 2 ? i : (guint64) g_test_rand_int_range(0, G_MAXINT32) + G_MAXINT32; + + g_assert_true(df_rand_dbus_objpath_string(&str, iteration) == 0); + g_assert_nonnull(str); + } + + /* Test certain specific/problematic cases */ + g_autoptr(gchar) str = NULL; + + g_assert_true(df_rand_dbus_objpath_string(&str, df_fuzz_get_buffer_length() - 2) == 0); + g_assert_nonnull(str); +} + +static void test_df_rand_dbus_signature_string(void) +{ + for (guint32 i = 0; i < RAND_TEST_ITERATIONS; i++) { + g_autoptr(gchar) str = NULL; + /* Test the "upper" guint64 interval in the second half of the iterations */ + guint64 iteration = i < RAND_TEST_ITERATIONS / 2 ? i : (guint64) g_test_rand_int_range(0, G_MAXINT32) + G_MAXINT32; + + g_assert_true(df_rand_dbus_signature_string(&str, iteration) == 0); + g_assert_true(g_variant_is_signature(str)); + g_assert_nonnull(str); + } +} + +static void test_df_rand_GVariant(void) +{ + for (guint32 i = 0; i < RAND_TEST_ITERATIONS; i++) { + g_autoptr(GVariant) variant = NULL; + /* Test the "upper" guint64 interval in the second half of the iterations */ + guint64 iteration = i < RAND_TEST_ITERATIONS / 2 ? i : (guint64) g_test_rand_int_range(0, G_MAXINT32) + G_MAXINT32; + + g_assert_true(df_rand_GVariant(&variant, iteration) == 0); + g_assert_nonnull(variant); + } +} + +int main(int argc, char *argv[]) +{ + g_test_init(&argc, &argv, NULL); + /* Init our internal pseudo-random number generators */ + df_rand_init(); + + g_test_add_func("/df_rand/df_rand_unichar", test_df_rand_unichar); + g_test_add_func("/df_rand/df_rand_string", test_df_rand_string); + g_test_add_func("/df_rand/df_rand_dbus_objpath_string", test_df_rand_dbus_objpath_string); + g_test_add_func("/df_rand/df_rand_dbus_signature_string", test_df_rand_dbus_signature_string); + g_test_add_func("/df_rand/df_rand_GVariant", test_df_rand_GVariant); + + return g_test_run(); +} From 60590237ea7ccde902fb31c00dce2fceda824ce0 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Mon, 4 Jul 2022 21:43:34 +0200 Subject: [PATCH 3/5] test: set -o pipefail To catch issues like : ``` + dfuzzer --max-iterations=10 -v -n org.freedesktop.systemd1 -o /org/freedesktop/systemd1 -i org.freedesktop.DBus.Peer -t Ping Loading suppressions from file './dfuzzer.conf' Found suppressions for bus: 'org.freedesktop.systemd1' Invalid suppression string ':::Ping' ================================================================= ==3465==ERROR: LeakSanitizer: detected memory leaks Direct leak of 32 byte(s) in 1 object(s) allocated from: #0 0x7f1b05122a06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153 #1 0x5611999f61ae in df_suppression_load ../src/suppression.c:122 #2 0x5611999e936a in main ../src/dfuzzer.c:898 #3 0x7f1b04170082 in __libc_start_main ../csu/libc-start.c:308 Indirect leak of 5 byte(s) in 1 object(s) allocated from: #0 0x7f1b050ab3ed in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cc:445 #1 0x5611999f6359 in df_suppression_load ../src/suppression.c:136 #2 0x5611999e936a in main ../src/dfuzzer.c:898 #3 0x7f1b04170082 in __libc_start_main ../csu/libc-start.c:308 Indirect leak of 1 byte(s) in 1 object(s) allocated from: #0 0x7f1b050ab3ed in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cc:445 #1 0x5611999f66a4 in df_suppression_load ../src/suppression.c:149 #2 0x5611999e936a in main ../src/dfuzzer.c:898 #3 0x7f1b04170082 in __libc_start_main ../csu/libc-start.c:308 SUMMARY: AddressSanitizer: 38 byte(s) leaked in 3 allocation(s). .github/workflows/run-tests.sh: line 122: 3465 Aborted (core dumped) "${dfuzzer[@]}" -v -n org.freedesktop.systemd1 -o /org/freedesktop/systemd1 -i org.freedesktop.DBus.Peer -t Ping ``` Suggested in https://github.com/dbus-fuzzer/dfuzzer/pull/117#issuecomment-1174124000 --- .github/workflows/run-tests.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/run-tests.sh b/.github/workflows/run-tests.sh index 1ae9517..d025dd3 100755 --- a/.github/workflows/run-tests.sh +++ b/.github/workflows/run-tests.sh @@ -1,6 +1,7 @@ #!/bin/bash set -ex +set -o pipefail ninja -C ./build test From e341394643437cf46be2a6c4225d8131eec1efdc Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Mon, 4 Jul 2022 23:00:01 +0200 Subject: [PATCH 4/5] rand: accept seed for rand()/srandom() from the outside So we can use reproducible seeds in tests to make reproducing test crashes easier. --- src/dfuzzer.c | 2 +- src/rand.c | 6 +++--- src/rand.h | 6 +----- test/test-rand.c | 12 ++++++++++-- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/dfuzzer.c b/src/dfuzzer.c index 1ba2819..9034f2a 100644 --- a/src/dfuzzer.c +++ b/src/dfuzzer.c @@ -231,7 +231,7 @@ static int df_fuzz(GDBusConnection *dcon, const char *name, const char *object, int rv = DF_BUS_OK; // initialization of random module - df_rand_init(); + df_rand_init(time(NULL)); // Sanity check fuzzing target if (isempty(name) || isempty(object) || isempty(interface)) { diff --git a/src/rand.c b/src/rand.c index 281abd2..8632868 100644 --- a/src/rand.c +++ b/src/rand.c @@ -39,10 +39,10 @@ static struct external_dictionary df_external_dictionary; * numbers generators. * @param buf_size Maximum buffer size for generated strings (in Bytes) */ -void df_rand_init() +void df_rand_init(unsigned int seed) { - srand(time(NULL)); // for int rand() - srandom(time(NULL)); // for long int random() + srand(seed); + srandom(seed); } int df_rand_load_external_dictionary(const char *filename) diff --git a/src/rand.h b/src/rand.h index 247f11d..c7ee8dc 100644 --- a/src/rand.h +++ b/src/rand.h @@ -30,11 +30,7 @@ struct external_dictionary { char **strings; }; -/** - * @function Initializes global flag variables and seeds pseudo-random - * numbers generators. - */ -void df_rand_init(); +void df_rand_init(unsigned int seed); int df_rand_load_external_dictionary(const char *filename); GVariant *df_generate_random_basic(const GVariantType *type, guint64 iteration); diff --git a/test/test-rand.c b/test/test-rand.c index 3bbd4ec..11a931f 100644 --- a/test/test-rand.c +++ b/test/test-rand.c @@ -109,8 +109,16 @@ static void test_df_rand_GVariant(void) int main(int argc, char *argv[]) { g_test_init(&argc, &argv, NULL); - /* Init our internal pseudo-random number generators */ - df_rand_init(); + /* Init our internal pseudo-random number generators + * + * Since we can't access the g_test_*() seed directly, let's use one + * of the g_test_rand*() functions that generate reproducible numbers + * (using the internal seed), so a possible test crash can be later + * reproduced using the `--seed xxx` option + * + * See: https://docs.gtk.org/glib/func.test_rand_int.html + * */ + df_rand_init(g_test_rand_int()); g_test_add_func("/df_rand/df_rand_unichar", test_df_rand_unichar); g_test_add_func("/df_rand/df_rand_string", test_df_rand_string); From ea2173744ee4e0d3b2e11fe2a6ee283980eca771 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Mon, 4 Jul 2022 23:03:29 +0200 Subject: [PATCH 5/5] packit: run the unit tests as well --- .packit.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.packit.yaml b/.packit.yaml index 9742f94..8123414 100644 --- a/.packit.yaml +++ b/.packit.yaml @@ -20,6 +20,7 @@ actions: # Drop the "sources" file so rebase-helper doesn't think we're a dist-git - "rm -fv .packit_rpm/sources" - sed -i '1 i%define _unpackaged_files_terminate_build 0' .packit_rpm/dfuzzer.spec + - sed -i '/^%meson$/a%meson_test' .packit_rpm/dfuzzer.spec - sed -i 's/^%meson$/%meson --werror -Ddfuzzer-test-server=true/' .packit_rpm/dfuzzer.spec jobs: