Merge lp:~jamesodhunt/upstart/bug-1234898 into lp:upstart

Proposed by James Hunt
Status: Merged
Merged at revision: 1541
Proposed branch: lp:~jamesodhunt/upstart/bug-1234898
Merge into: lp:upstart
Diff against target: 244 lines (+122/-73)
2 files modified
ChangeLog (+8/-0)
extra/upstart-local-bridge.c (+114/-73)
To merge this branch: bzr merge lp:~jamesodhunt/upstart/bug-1234898
Reviewer Review Type Date Requested Status
Dimitri John Ledkov Approve
Review via email: mp+189296@code.launchpad.net
To post a comment you must log in.
lp:~jamesodhunt/upstart/bug-1234898 updated
1541. By James Hunt

* Oops.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

so it's mostly refactor + handle pairs.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'ChangeLog'
--- ChangeLog 2013-10-03 10:10:56 +0000
+++ ChangeLog 2013-10-04 12:34:01 +0000
@@ -1,3 +1,11 @@
12013-10-04 James Hunt <james.hunt@ubuntu.com>
2
3 * extra/upstart-local-bridge.c:
4 - socket_reader():
5 - Handle event emission in new function emit_event().
6 - Check that data is printable.
7 - Allow input to be a set of pairs (LP: #1234898).
8
12013-10-03 James Hunt <james.hunt@ubuntu.com>92013-10-03 James Hunt <james.hunt@ubuntu.com>
210
3 * util/tests/test_utmp.c: Update remaining tests to pause11 * util/tests/test_utmp.c: Update remaining tests to pause
412
=== 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 12:34:01 +0000
@@ -30,6 +30,7 @@
30#include <string.h>30#include <string.h>
31#include <syslog.h>31#include <syslog.h>
32#include <unistd.h>32#include <unistd.h>
33#include <ctype.h>
3334
34#include <nih/macros.h>35#include <nih/macros.h>
35#include <nih/alloc.h>36#include <nih/alloc.h>
@@ -118,6 +119,8 @@
118119
119static void emit_event_error (void *data, NihDBusMessage *message);120static void emit_event_error (void *data, NihDBusMessage *message);
120121
122static void emit_event (ClientConnection *client, const char *pair, size_t len);
123
121static void signal_handler (void *data, NihSignal *signal);124static void signal_handler (void *data, NihSignal *signal);
122125
123static void cleanup (void);126static void cleanup (void);
@@ -589,92 +592,65 @@
589 const char *buf,592 const char *buf,
590 size_t len)593 size_t len)
591{594{
592 DBusPendingCall *pending_call;595 nih_local char *pairs = NULL;
593 nih_local char **env = NULL;596 char *pair;
594 nih_local char *var = NULL;
595 size_t used_len = 0;597 size_t used_len = 0;
596 int i;598 size_t i;
599
600 /* Ignore messages that are too short.
601 * (minimum message is of form "a=").
602 */
603 size_t min_len = 2;
597604
598 nih_assert (sock);605 nih_assert (sock);
599 nih_assert (client);606 nih_assert (client);
600 nih_assert (io);607 nih_assert (io);
601 nih_assert (buf);608 nih_assert (buf);
602609
603 /* Ignore messages that are too short */610 if (len < min_len)
604 if (len < 2)611 goto error;
605 goto error;612
606613 pairs = nih_strdup (NULL, buf);
607 /* Ensure the data is a name=value pair */614 if (! pairs)
608 if (! strchr (buf, '=') || buf[0] == '=')615 return;
609 goto error;616
610617 for (pair = strsep (&pairs, "\n");
611 /* Remove line endings */618 pair;
612 for (i = 0, used_len = len; i < 2; i++) {619 pair = strsep (&pairs, "\n")) {
613 if (buf[used_len-1] == '\n' || buf[used_len-1] == '\r')620
621 used_len = strlen (pair);
622
623 if (used_len < min_len)
624 continue;
625
626 /* Ensure the data is a 'name=value' pair */
627 if (! strchr (pair, '=') || pair[0] == '=')
628 continue;
629
630 /* Remove extraneous line ending */
631 if (pair[used_len-1] == '\r') {
632 pair[used_len-1] = '\0';
614 used_len--;633 used_len--;
615 else634 }
616 break;635
617 }636 /* Ignore invalid input */
618637 for (i = 0; i < used_len; i++) {
619 /* Second check to ensure overly short messages are ignored */638 if (! isprint (pair[i]) && ! isspace (pair[i]))
620 if (used_len < 2)639 continue;
621 goto error;640 }
622641
623 /* Construct the event environment.642 /* Yet another check to ensure overly short messages are ignored
624 *643 * (required since we may have adjusted used_len
625 * Note that although the client could conceivably specify one644 */
626 * of the variables below _itself_, if the intent is malicious645 if (used_len < min_len)
627 * it will be thwarted since although the following example646 continue;
628 * event is valid...647
629 *648 emit_event (client, pair, used_len);
630 * foo BAR=BAZ BAR=MALICIOUS
631 *
632 * ... environment variable matching only happens for the first
633 * occurence of a variable. In summary, a malicious client
634 * cannot spoof the standard variables we set.
635 */
636 env = NIH_MUST (nih_str_array_new (NULL));
637
638 /* Specify type to allow for other types to be added in the future */
639 var = NIH_MUST (nih_sprintf (NULL, "SOCKET_TYPE=%s", socket_type));
640 NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
641
642 var = NIH_MUST (nih_sprintf (NULL, "SOCKET_VARIANT=%s",
643 sock->sun_addr.sun_path[0] ? "named" : "abstract"));
644 NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
645
646 var = NIH_MUST (nih_sprintf (NULL, "CLIENT_UID=%u", (unsigned int)client->ucred.uid));
647 NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
648
649 var = NIH_MUST (nih_sprintf (NULL, "CLIENT_GID=%u", (unsigned int)client->ucred.gid));
650 NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
651
652 var = NIH_MUST (nih_sprintf (NULL, "CLIENT_PID=%u", (unsigned int)client->ucred.pid));
653 NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
654
655 var = NIH_MUST (nih_sprintf (NULL, "PATH=%s", socket_path));
656 NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
657
658 /* Add the name=value pair */
659 NIH_MUST (nih_str_array_addn (&env, NULL, NULL, buf, used_len));
660
661 pending_call = upstart_emit_event (upstart,
662 event_name, env, FALSE,
663 NULL, emit_event_error, NULL,
664 NIH_DBUS_TIMEOUT_NEVER);
665
666 if (! pending_call) {
667 NihError *err;
668 err = nih_error_get ();
669 nih_warn ("%s", err->message);
670 nih_free (err);
671 }649 }
672650
673 /* Consume the entire length */651 /* Consume the entire length */
674 nih_io_buffer_shrink (io->recv_buf, len);652 nih_io_buffer_shrink (io->recv_buf, len);
675653
676 dbus_pending_call_unref (pending_call);
677
678 return;654 return;
679655
680error:656error:
@@ -803,3 +779,68 @@
803 nih_warn ("%s", err->message);779 nih_warn ("%s", err->message);
804 nih_free (err);780 nih_free (err);
805}781}
782
783static void
784emit_event (ClientConnection *client,
785 const char *pair,
786 size_t len)
787{
788 DBusPendingCall *pending_call;
789 nih_local char **env = NULL;
790 nih_local char *var = NULL;
791
792 nih_assert (client);
793 nih_assert (pair);
794 nih_assert (len);
795 /* Construct the event environment.
796 *
797 * Note that although the client could conceivably specify one
798 * of the variables below _itself_, if the intent is malicious
799 * it will be thwarted since although the following example
800 * event is valid...
801 *
802 * foo BAR=BAZ BAR=MALICIOUS
803 *
804 * ... environment variable matching only happens for the first
805 * occurence of a variable. In summary, a malicious client
806 * cannot spoof the standard variables we set.
807 */
808 env = NIH_MUST (nih_str_array_new (NULL));
809
810 /* Specify type to allow for other types to be added in the future */
811 var = NIH_MUST (nih_sprintf (NULL, "SOCKET_TYPE=%s", socket_type));
812 NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
813
814 var = NIH_MUST (nih_sprintf (NULL, "SOCKET_VARIANT=%s",
815 sock->sun_addr.sun_path[0] ? "named" : "abstract"));
816 NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
817
818 var = NIH_MUST (nih_sprintf (NULL, "CLIENT_UID=%u", (unsigned int)client->ucred.uid));
819 NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
820
821 var = NIH_MUST (nih_sprintf (NULL, "CLIENT_GID=%u", (unsigned int)client->ucred.gid));
822 NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
823
824 var = NIH_MUST (nih_sprintf (NULL, "CLIENT_PID=%u", (unsigned int)client->ucred.pid));
825 NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
826
827 var = NIH_MUST (nih_sprintf (NULL, "PATH=%s", socket_path));
828 NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
829
830 /* Add the name=value pair */
831 NIH_MUST (nih_str_array_addn (&env, NULL, NULL, pair, len));
832
833 pending_call = upstart_emit_event (upstart,
834 event_name, env, FALSE,
835 NULL, emit_event_error, NULL,
836 NIH_DBUS_TIMEOUT_NEVER);
837
838 if (! pending_call) {
839 NihError *err;
840 err = nih_error_get ();
841 nih_warn ("%s", err->message);
842 nih_free (err);
843 }
844
845 dbus_pending_call_unref (pending_call);
846}

Subscribers

People subscribed via source and target branches