Merge authors: James Hunt (jamesodhunt) Related merge proposals: https://code.launchpad.net/~jamesodhunt/upstart/bug-1234898/+merge/189296 proposed by: James Hunt (jamesodhunt) review: Approve - Dmitrijs Ledkovs (xnox) ------------------------------------------------------------ revno: 1541 [merge] committer: Dmitrijs Ledkovs <[email protected]> branch nick: upstart timestamp: Fri 2013-10-04 15:22:39 +0100 message: merge lp:~jamesodhunt/upstart/bug-1234898 modified: ChangeLog extra/upstart-local-bridge.c
-- lp:upstart https://code.launchpad.net/~upstart-devel/upstart/trunk Your team Upstart Reviewers is subscribed to branch lp:upstart. To unsubscribe from this branch go to https://code.launchpad.net/~upstart-devel/upstart/trunk/+edit-subscription
=== modified file 'ChangeLog' --- ChangeLog 2013-10-03 10:10:56 +0000 +++ ChangeLog 2013-10-04 12:33:07 +0000 @@ -1,3 +1,11 @@ +2013-10-04 James Hunt <[email protected]> + + * extra/upstart-local-bridge.c: + - socket_reader(): + - Handle event emission in new function emit_event(). + - Check that data is printable. + - Allow input to be a set of pairs (LP: #1234898). + 2013-10-03 James Hunt <[email protected]> * util/tests/test_utmp.c: Update remaining tests to pause === modified file 'extra/upstart-local-bridge.c' --- extra/upstart-local-bridge.c 2013-08-21 11:25:14 +0000 +++ extra/upstart-local-bridge.c 2013-10-04 11:12:44 +0000 @@ -30,6 +30,7 @@ #include <string.h> #include <syslog.h> #include <unistd.h> +#include <ctype.h> #include <nih/macros.h> #include <nih/alloc.h> @@ -118,6 +119,8 @@ static void emit_event_error (void *data, NihDBusMessage *message); +static void emit_event (ClientConnection *client, const char *pair, size_t len); + static void signal_handler (void *data, NihSignal *signal); static void cleanup (void); @@ -589,92 +592,65 @@ const char *buf, size_t len) { - DBusPendingCall *pending_call; - nih_local char **env = NULL; - nih_local char *var = NULL; + nih_local char *pairs = NULL; + char *pair; size_t used_len = 0; - int i; + size_t i; + + /* Ignore messages that are too short. + * (minimum message is of form "a="). + */ + size_t min_len = 2; nih_assert (sock); nih_assert (client); nih_assert (io); nih_assert (buf); - /* Ignore messages that are too short */ - if (len < 2) - goto error; - - /* Ensure the data is a name=value pair */ - if (! strchr (buf, '=') || buf[0] == '=') - goto error; - - /* Remove line endings */ - for (i = 0, used_len = len; i < 2; i++) { - if (buf[used_len-1] == '\n' || buf[used_len-1] == '\r') + if (len < min_len) + goto error; + + pairs = nih_strdup (NULL, buf); + if (! pairs) + return; + + for (pair = strsep (&pairs, "\n"); + pair; + pair = strsep (&pairs, "\n")) { + + used_len = strlen (pair); + + if (used_len < min_len) + continue; + + /* Ensure the data is a 'name=value' pair */ + if (! strchr (pair, '=') || pair[0] == '=') + continue; + + /* Remove extraneous line ending */ + if (pair[used_len-1] == '\r') { + pair[used_len-1] = '\0'; used_len--; - else - break; - } - - /* Second check to ensure overly short messages are ignored */ - if (used_len < 2) - goto error; - - /* Construct the event environment. - * - * Note that although the client could conceivably specify one - * of the variables below _itself_, if the intent is malicious - * it will be thwarted since although the following example - * event is valid... - * - * foo BAR=BAZ BAR=MALICIOUS - * - * ... environment variable matching only happens for the first - * occurence of a variable. In summary, a malicious client - * cannot spoof the standard variables we set. - */ - env = NIH_MUST (nih_str_array_new (NULL)); - - /* Specify type to allow for other types to be added in the future */ - var = NIH_MUST (nih_sprintf (NULL, "SOCKET_TYPE=%s", socket_type)); - NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var)); - - var = NIH_MUST (nih_sprintf (NULL, "SOCKET_VARIANT=%s", - sock->sun_addr.sun_path[0] ? "named" : "abstract")); - NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var)); - - var = NIH_MUST (nih_sprintf (NULL, "CLIENT_UID=%u", (unsigned int)client->ucred.uid)); - NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var)); - - var = NIH_MUST (nih_sprintf (NULL, "CLIENT_GID=%u", (unsigned int)client->ucred.gid)); - NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var)); - - var = NIH_MUST (nih_sprintf (NULL, "CLIENT_PID=%u", (unsigned int)client->ucred.pid)); - NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var)); - - var = NIH_MUST (nih_sprintf (NULL, "PATH=%s", socket_path)); - NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var)); - - /* Add the name=value pair */ - NIH_MUST (nih_str_array_addn (&env, NULL, NULL, buf, used_len)); - - pending_call = upstart_emit_event (upstart, - event_name, env, FALSE, - NULL, emit_event_error, NULL, - NIH_DBUS_TIMEOUT_NEVER); - - if (! pending_call) { - NihError *err; - err = nih_error_get (); - nih_warn ("%s", err->message); - nih_free (err); + } + + /* Ignore invalid input */ + for (i = 0; i < used_len; i++) { + if (! isprint (pair[i]) && ! isspace (pair[i])) + continue; + } + + /* Yet another check to ensure overly short messages are ignored + * (required since we may have adjusted used_len + */ + if (used_len < min_len) + continue; + + emit_event (client, pair, used_len); } /* Consume the entire length */ nih_io_buffer_shrink (io->recv_buf, len); - dbus_pending_call_unref (pending_call); - return; error: @@ -803,3 +779,68 @@ nih_warn ("%s", err->message); nih_free (err); } + +static void +emit_event (ClientConnection *client, + const char *pair, + size_t len) +{ + DBusPendingCall *pending_call; + nih_local char **env = NULL; + nih_local char *var = NULL; + + nih_assert (client); + nih_assert (pair); + nih_assert (len); + /* Construct the event environment. + * + * Note that although the client could conceivably specify one + * of the variables below _itself_, if the intent is malicious + * it will be thwarted since although the following example + * event is valid... + * + * foo BAR=BAZ BAR=MALICIOUS + * + * ... environment variable matching only happens for the first + * occurence of a variable. In summary, a malicious client + * cannot spoof the standard variables we set. + */ + env = NIH_MUST (nih_str_array_new (NULL)); + + /* Specify type to allow for other types to be added in the future */ + var = NIH_MUST (nih_sprintf (NULL, "SOCKET_TYPE=%s", socket_type)); + NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var)); + + var = NIH_MUST (nih_sprintf (NULL, "SOCKET_VARIANT=%s", + sock->sun_addr.sun_path[0] ? "named" : "abstract")); + NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var)); + + var = NIH_MUST (nih_sprintf (NULL, "CLIENT_UID=%u", (unsigned int)client->ucred.uid)); + NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var)); + + var = NIH_MUST (nih_sprintf (NULL, "CLIENT_GID=%u", (unsigned int)client->ucred.gid)); + NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var)); + + var = NIH_MUST (nih_sprintf (NULL, "CLIENT_PID=%u", (unsigned int)client->ucred.pid)); + NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var)); + + var = NIH_MUST (nih_sprintf (NULL, "PATH=%s", socket_path)); + NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var)); + + /* Add the name=value pair */ + NIH_MUST (nih_str_array_addn (&env, NULL, NULL, pair, len)); + + pending_call = upstart_emit_event (upstart, + event_name, env, FALSE, + NULL, emit_event_error, NULL, + NIH_DBUS_TIMEOUT_NEVER); + + if (! pending_call) { + NihError *err; + err = nih_error_get (); + nih_warn ("%s", err->message); + nih_free (err); + } + + dbus_pending_call_unref (pending_call); +}
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
