diff options
author | Jakub Hrozek <jhrozek@redhat.com> | 2015-11-06 20:54:13 +0100 |
---|---|---|
committer | Jakub Hrozek <jhrozek@redhat.com> | 2015-11-09 15:33:46 +0100 |
commit | b9a9bc90c8be499c84533b3687aae3dfe835a117 (patch) | |
tree | c36070d2edd30b250469ef5d8a87e4790651f046 /src | |
parent | 81ca4c578d77659d32ccf71050a1b4d6686b42a3 (diff) | |
download | sssd-b9a9bc90c8be499c84533b3687aae3dfe835a117.tar.gz sssd-b9a9bc90c8be499c84533b3687aae3dfe835a117.tar.xz sssd-b9a9bc90c8be499c84533b3687aae3dfe835a117.zip |
sbus: Check string arguments for valid UTF-8 strings
libdbus abort()s when a string argument is not valid UTF-8. Since the
arguments sometimes come from untrusted sources, it's better to check
the string validity explicitly.
Diffstat (limited to 'src')
-rw-r--r-- | src/sbus/sssd_dbus_request.c | 40 | ||||
-rw-r--r-- | src/tests/sbus_tests.c | 47 |
2 files changed, 86 insertions, 1 deletions
diff --git a/src/sbus/sssd_dbus_request.c b/src/sbus/sssd_dbus_request.c index 552bb5da8..d2f2bd535 100644 --- a/src/sbus/sssd_dbus_request.c +++ b/src/sbus/sssd_dbus_request.c @@ -96,15 +96,54 @@ int sbus_request_finish(struct sbus_request *dbus_req, return talloc_free(dbus_req); } +static int sbus_request_valist_check(va_list va, int first_arg_type) +{ + int ret = EOK; + int type; + va_list va_check; + const DBusBasicValue *value; + dbus_bool_t ok; + + va_copy(va_check, va); + + type = first_arg_type; + while (type != DBUS_TYPE_INVALID) { + value = va_arg(va_check, const DBusBasicValue*); + + if (type == DBUS_TYPE_STRING) { + ok = dbus_validate_utf8(value->str, NULL); + if (!ok) { + DEBUG(SSSDBG_CRIT_FAILURE, + "String argument is not valid UTF-8\n"); + ret = EINVAL; + break; + } + } + type = va_arg(va_check, int); + } + + va_end(va_check); + return ret; +} + int sbus_request_return_and_finish(struct sbus_request *dbus_req, int first_arg_type, ...) { DBusMessage *reply; + DBusError error = DBUS_ERROR_INIT; dbus_bool_t dbret; va_list va; int ret; + va_start(va, first_arg_type); + ret = sbus_request_valist_check(va, first_arg_type); + if (ret != EOK) { + va_end(va); + dbus_set_error_const(&error, DBUS_ERROR_INVALID_ARGS, INTERNAL_ERROR); + return sbus_request_fail_and_finish(dbus_req, &error); + } + reply = dbus_message_new_method_return(dbus_req->message); if (!reply) { DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory allocating DBus message\n"); @@ -112,7 +151,6 @@ int sbus_request_return_and_finish(struct sbus_request *dbus_req, return ENOMEM; } - va_start(va, first_arg_type); dbret = dbus_message_append_args_valist(reply, first_arg_type, va); va_end(va); diff --git a/src/tests/sbus_tests.c b/src/tests/sbus_tests.c index cae82cfff..7da3c64c1 100644 --- a/src/tests/sbus_tests.c +++ b/src/tests/sbus_tests.c @@ -43,6 +43,7 @@ #define PILOT_IFACE "test.Pilot" #define PILOT_BLINK "Blink" #define PILOT_EAT "Eat" +#define PILOT_CRASH "Crash" #define PILOT_IFACE_INTROSPECT \ "<!DOCTYPE node PUBLIC \"-//freedesktop//DTD D-BUS Object Introspection 1.0//EN\"\n" \ @@ -72,6 +73,7 @@ " <interface name=\"test.Pilot\">\n" \ " <method name=\"Blink\" />\n" \ " <method name=\"Eat\" />\n" \ + " <method name=\"Crash\" />\n" \ " </interface>\n" \ "</node>\n" @@ -80,6 +82,7 @@ struct pilot_vtable { struct sbus_vtable vtable; sbus_msg_handler_fn Blink; sbus_msg_handler_fn Eat; + sbus_msg_handler_fn Crash; }; const struct sbus_method_meta pilot_methods[] = { @@ -97,6 +100,13 @@ const struct sbus_method_meta pilot_methods[] = { offsetof(struct pilot_vtable, Eat), NULL }, + { + PILOT_CRASH, /* method name */ + NULL, /* in args: manually parsed */ + NULL, /* out args: manually parsed */ + offsetof(struct pilot_vtable, Crash), + NULL + }, { NULL, } }; @@ -169,10 +179,21 @@ static int eat_handler(struct sbus_request *req, void *data) return sbus_request_return_and_finish(req, DBUS_TYPE_INVALID); } +static int crash_handler(struct sbus_request *req, void *data) +{ + /* Pilot crashes by returning a malformed UTF-8 string */ + const char *invalid = "ad\351la\357d"; + + return sbus_request_return_and_finish(req, + DBUS_TYPE_STRING, &invalid, + DBUS_TYPE_INVALID); +} + struct pilot_vtable pilot_impl = { { &pilot_meta, 0 }, .Blink = blink_handler, .Eat = eat_handler, + .Crash = crash_handler, }; static int pilot_test_server_init(struct sbus_connection *server, void *unused) @@ -304,6 +325,31 @@ START_TEST(test_request_parse_bad_args) } END_TEST +START_TEST(test_request_dontcrash) +{ + TALLOC_CTX *ctx; + DBusConnection *client; + DBusError error = DBUS_ERROR_INIT; + DBusMessage *reply; + + ctx = talloc_new(NULL); + client = test_dbus_setup_mock(ctx, NULL, pilot_test_server_init, NULL); + + reply = test_dbus_call_sync(client, + "/test/leela", + PILOT_IFACE, + PILOT_CRASH, + &error, + DBUS_TYPE_INVALID); /* bad agruments */ + ck_assert(reply == NULL); + ck_assert(dbus_error_is_set(&error)); + ck_assert(dbus_error_has_name(&error, DBUS_ERROR_INVALID_ARGS)); + dbus_error_free(&error); + + talloc_free(ctx); +} +END_TEST + START_TEST(test_introspection) { TALLOC_CTX *ctx; @@ -387,6 +433,7 @@ TCase *create_sbus_tests(void) tcase_add_test(tc, test_raw_handler); tcase_add_test(tc, test_request_parse_ok); tcase_add_test(tc, test_request_parse_bad_args); + tcase_add_test(tc, test_request_dontcrash); tcase_add_test(tc, test_introspection); tcase_add_test(tc, test_sbus_new_error); tcase_add_test(tc, test_utf8_check); |