From 6e1ee192dd282cfb63537841ec52314a4f5eaac5 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Tue, 10 May 2022 10:55:24 +0200 Subject: [PATCH] fuzz: respect the `org.freedesktop.DBus.Method.NoReply` annotation So we don't expect a reply from a method which intentionally doesn't respond. --- .github/workflows/run-tests.sh | 9 +++++++++ src/dfuzzer-test-server.c | 8 ++++++-- src/dfuzzer.c | 1 + src/dfuzzer.conf | 1 - src/fuzz.c | 4 +++- src/fuzz.h | 1 + src/introspection.c | 13 +++++++++++++ src/introspection.h | 1 + 8 files changed, 34 insertions(+), 4 deletions(-) diff --git a/.github/workflows/run-tests.sh b/.github/workflows/run-tests.sh index 548ebb8..7988844 100755 --- a/.github/workflows/run-tests.sh +++ b/.github/workflows/run-tests.sh @@ -7,6 +7,11 @@ if [[ "$TYPE" == valgrind ]]; then dfuzzer=("valgrind" "--leak-check=full" "--show-leak-kinds=definite" "--errors-for-leak-kinds=definite" "--error-exitcode=42" "dfuzzer") fi +# CI specific suppressions for issues already fixed in upstream +sudo sed -i '/\[org.freedesktop.systemd1\]/a \ +org.freedesktop.systemd1.Manager:Reexecute Fixed by https://github.com/systemd/systemd/pull/23328 \ +' /etc/dfuzzer.conf + sudo systemctl daemon-reload # Test if we can list activatable dbus services as well @@ -35,6 +40,10 @@ EOF "${dfuzzer[@]}" -f inputs.txt -s -v -n org.freedesktop.dfuzzerServer -o /org/freedesktop/dfuzzerObject -i org.freedesktop.dfuzzerInterface -t df_crash_on_leeroy && false rm -f inputs.txt +# Test if we respect the org.freedesktop.DBus.Method.NoReply annotation +"${dfuzzer[@]}" -s -v -n org.freedesktop.dfuzzerServer -o /org/freedesktop/dfuzzerObject -i org.freedesktop.dfuzzerInterface -t df_noreply && false +"${dfuzzer[@]}" -s -v -n org.freedesktop.dfuzzerServer -o /org/freedesktop/dfuzzerObject -i org.freedesktop.dfuzzerInterface -t df_noreply_expected + sudo systemctl stop dfuzzer-test-server # dfuzzer should return 0 by default when services it tests time out diff --git a/src/dfuzzer-test-server.c b/src/dfuzzer-test-server.c index edda0a4..7547a58 100644 --- a/src/dfuzzer-test-server.c +++ b/src/dfuzzer-test-server.c @@ -60,6 +60,10 @@ static const gchar introspection_xml[] = " " " " " " +" " +" " +" " +" " " " " " " " @@ -127,8 +131,8 @@ static void handle_method_call( g_dbus_method_invocation_return_value(invocation, g_variant_new("()")); } else if (g_strcmp0(method_name, "df_hang") == 0) pause(); - else if (g_strcmp0(method_name, "df_noreply") == 0) - return; + else if (g_strcmp0(method_name, "df_noreply") == 0 || g_strcmp0(method_name, "df_noreply_expected") == 0) + g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.NoReply", "org.freedesktop.DBus.Error.NoReply"); else if (g_strcmp0(method_name, "df_complex_sig_1") == 0) { gchar *str = NULL; unsigned u; diff --git a/src/dfuzzer.c b/src/dfuzzer.c index f7d353e..0b55f3f 100644 --- a/src/dfuzzer.c +++ b/src/dfuzzer.c @@ -533,6 +533,7 @@ int df_fuzz(GDBusConnection *dcon, const char *name, const char *object, const c dbus_method.name = strdup(m->name); dbus_method.signature = df_method_get_full_signature(m); dbus_method.returns_value = !!*(m->out_args); + dbus_method.expect_reply = df_method_returns_reply(m); dbus_method.fuzz_on_str_len = (strstr(dbus_method.signature, "s") || strstr(dbus_method.signature, "v")); // tests for method diff --git a/src/dfuzzer.conf b/src/dfuzzer.conf index 6c91ca3..86b3a43 100644 --- a/src/dfuzzer.conf +++ b/src/dfuzzer.conf @@ -31,7 +31,6 @@ org.freedesktop.systemd1.Manager:Halt destructive org.freedesktop.systemd1.Manager:KExec destructive org.freedesktop.systemd1.Manager:PowerOff destructive org.freedesktop.systemd1.Manager:Reboot destructive -org.freedesktop.systemd1.Manager:Reexecute FIXME: disconnects systemd from the bus org.freedesktop.systemd1.Manager:RefUnit destructive org.freedesktop.systemd1.Manager:UnrefUnit destructive Freeze destructive diff --git a/src/fuzz.c b/src/fuzz.c index 1775b43..18adea9 100644 --- a/src/fuzz.c +++ b/src/fuzz.c @@ -591,7 +591,9 @@ static int df_fuzz_call_method(const struct df_dbus_method *method, GVariant *va if (dbus_error) { // if process does not respond if (strcmp(dbus_error, "org.freedesktop.DBus.Error.NoReply") == 0) - return -1; + /* If the method is annotated as "NoReply", don't consider + * not replying as an error */ + return method->expect_reply ? -1 : 0; else if (strcmp(dbus_error, "org.freedesktop.DBus.Error.Timeout") == 0) { sleep(10); // wait for tested process; processing // of longer inputs may take a longer time diff --git a/src/fuzz.h b/src/fuzz.h index 7414520..b964aa1 100644 --- a/src/fuzz.h +++ b/src/fuzz.h @@ -32,6 +32,7 @@ struct df_dbus_method { char *name; char *signature; gboolean returns_value; + gboolean expect_reply; int fuzz_on_str_len; }; diff --git a/src/introspection.c b/src/introspection.c index 9cd689e..6c0f5e3 100644 --- a/src/introspection.c +++ b/src/introspection.c @@ -102,3 +102,16 @@ char *df_method_get_full_signature(const GDBusMethodInfo *method) return r; } +gboolean df_method_returns_reply(const GDBusMethodInfo *method) +{ + const gchar *annotation_str; + + assert(method); + + annotation_str = g_dbus_annotation_info_lookup(method->annotations, + "org.freedesktop.DBus.Method.NoReply"); + if (!isempty(annotation_str) && g_strcmp0(annotation_str, "true") == 0) + return FALSE; + + return TRUE; +} diff --git a/src/introspection.h b/src/introspection.h index b4c7aa0..d681142 100644 --- a/src/introspection.h +++ b/src/introspection.h @@ -22,5 +22,6 @@ GDBusNodeInfo *df_get_interface_info(GDBusProxy *dproxy, const char *interface, GDBusInterfaceInfo **ret_iinfo); char *df_method_get_full_signature(const GDBusMethodInfo *method); +gboolean df_method_returns_reply(const GDBusMethodInfo *method); #endif