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

Proposed by James Hunt on 2013-10-04
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 2013-10-04 Approve on 2013-10-04
Review via email: mp+189296@code.launchpad.net
To post a comment you must log in.
lp:~jamesodhunt/upstart/bug-1234898 updated on 2013-10-04
1541. By James Hunt on 2013-10-04

* Oops.

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
1=== modified file 'ChangeLog'
2--- ChangeLog 2013-10-03 10:10:56 +0000
3+++ ChangeLog 2013-10-04 12:34:01 +0000
4@@ -1,3 +1,11 @@
5+2013-10-04 James Hunt <james.hunt@ubuntu.com>
6+
7+ * extra/upstart-local-bridge.c:
8+ - socket_reader():
9+ - Handle event emission in new function emit_event().
10+ - Check that data is printable.
11+ - Allow input to be a set of pairs (LP: #1234898).
12+
13 2013-10-03 James Hunt <james.hunt@ubuntu.com>
14
15 * util/tests/test_utmp.c: Update remaining tests to pause
16
17=== modified file 'extra/upstart-local-bridge.c'
18--- extra/upstart-local-bridge.c 2013-08-21 11:25:14 +0000
19+++ extra/upstart-local-bridge.c 2013-10-04 12:34:01 +0000
20@@ -30,6 +30,7 @@
21 #include <string.h>
22 #include <syslog.h>
23 #include <unistd.h>
24+#include <ctype.h>
25
26 #include <nih/macros.h>
27 #include <nih/alloc.h>
28@@ -118,6 +119,8 @@
29
30 static void emit_event_error (void *data, NihDBusMessage *message);
31
32+static void emit_event (ClientConnection *client, const char *pair, size_t len);
33+
34 static void signal_handler (void *data, NihSignal *signal);
35
36 static void cleanup (void);
37@@ -589,92 +592,65 @@
38 const char *buf,
39 size_t len)
40 {
41- DBusPendingCall *pending_call;
42- nih_local char **env = NULL;
43- nih_local char *var = NULL;
44+ nih_local char *pairs = NULL;
45+ char *pair;
46 size_t used_len = 0;
47- int i;
48+ size_t i;
49+
50+ /* Ignore messages that are too short.
51+ * (minimum message is of form "a=").
52+ */
53+ size_t min_len = 2;
54
55 nih_assert (sock);
56 nih_assert (client);
57 nih_assert (io);
58 nih_assert (buf);
59
60- /* Ignore messages that are too short */
61- if (len < 2)
62- goto error;
63-
64- /* Ensure the data is a name=value pair */
65- if (! strchr (buf, '=') || buf[0] == '=')
66- goto error;
67-
68- /* Remove line endings */
69- for (i = 0, used_len = len; i < 2; i++) {
70- if (buf[used_len-1] == '\n' || buf[used_len-1] == '\r')
71+ if (len < min_len)
72+ goto error;
73+
74+ pairs = nih_strdup (NULL, buf);
75+ if (! pairs)
76+ return;
77+
78+ for (pair = strsep (&pairs, "\n");
79+ pair;
80+ pair = strsep (&pairs, "\n")) {
81+
82+ used_len = strlen (pair);
83+
84+ if (used_len < min_len)
85+ continue;
86+
87+ /* Ensure the data is a 'name=value' pair */
88+ if (! strchr (pair, '=') || pair[0] == '=')
89+ continue;
90+
91+ /* Remove extraneous line ending */
92+ if (pair[used_len-1] == '\r') {
93+ pair[used_len-1] = '\0';
94 used_len--;
95- else
96- break;
97- }
98-
99- /* Second check to ensure overly short messages are ignored */
100- if (used_len < 2)
101- goto error;
102-
103- /* Construct the event environment.
104- *
105- * Note that although the client could conceivably specify one
106- * of the variables below _itself_, if the intent is malicious
107- * it will be thwarted since although the following example
108- * event is valid...
109- *
110- * foo BAR=BAZ BAR=MALICIOUS
111- *
112- * ... environment variable matching only happens for the first
113- * occurence of a variable. In summary, a malicious client
114- * cannot spoof the standard variables we set.
115- */
116- env = NIH_MUST (nih_str_array_new (NULL));
117-
118- /* Specify type to allow for other types to be added in the future */
119- var = NIH_MUST (nih_sprintf (NULL, "SOCKET_TYPE=%s", socket_type));
120- NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
121-
122- var = NIH_MUST (nih_sprintf (NULL, "SOCKET_VARIANT=%s",
123- sock->sun_addr.sun_path[0] ? "named" : "abstract"));
124- NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
125-
126- var = NIH_MUST (nih_sprintf (NULL, "CLIENT_UID=%u", (unsigned int)client->ucred.uid));
127- NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
128-
129- var = NIH_MUST (nih_sprintf (NULL, "CLIENT_GID=%u", (unsigned int)client->ucred.gid));
130- NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
131-
132- var = NIH_MUST (nih_sprintf (NULL, "CLIENT_PID=%u", (unsigned int)client->ucred.pid));
133- NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
134-
135- var = NIH_MUST (nih_sprintf (NULL, "PATH=%s", socket_path));
136- NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
137-
138- /* Add the name=value pair */
139- NIH_MUST (nih_str_array_addn (&env, NULL, NULL, buf, used_len));
140-
141- pending_call = upstart_emit_event (upstart,
142- event_name, env, FALSE,
143- NULL, emit_event_error, NULL,
144- NIH_DBUS_TIMEOUT_NEVER);
145-
146- if (! pending_call) {
147- NihError *err;
148- err = nih_error_get ();
149- nih_warn ("%s", err->message);
150- nih_free (err);
151+ }
152+
153+ /* Ignore invalid input */
154+ for (i = 0; i < used_len; i++) {
155+ if (! isprint (pair[i]) && ! isspace (pair[i]))
156+ continue;
157+ }
158+
159+ /* Yet another check to ensure overly short messages are ignored
160+ * (required since we may have adjusted used_len
161+ */
162+ if (used_len < min_len)
163+ continue;
164+
165+ emit_event (client, pair, used_len);
166 }
167
168 /* Consume the entire length */
169 nih_io_buffer_shrink (io->recv_buf, len);
170
171- dbus_pending_call_unref (pending_call);
172-
173 return;
174
175 error:
176@@ -803,3 +779,68 @@
177 nih_warn ("%s", err->message);
178 nih_free (err);
179 }
180+
181+static void
182+emit_event (ClientConnection *client,
183+ const char *pair,
184+ size_t len)
185+{
186+ DBusPendingCall *pending_call;
187+ nih_local char **env = NULL;
188+ nih_local char *var = NULL;
189+
190+ nih_assert (client);
191+ nih_assert (pair);
192+ nih_assert (len);
193+ /* Construct the event environment.
194+ *
195+ * Note that although the client could conceivably specify one
196+ * of the variables below _itself_, if the intent is malicious
197+ * it will be thwarted since although the following example
198+ * event is valid...
199+ *
200+ * foo BAR=BAZ BAR=MALICIOUS
201+ *
202+ * ... environment variable matching only happens for the first
203+ * occurence of a variable. In summary, a malicious client
204+ * cannot spoof the standard variables we set.
205+ */
206+ env = NIH_MUST (nih_str_array_new (NULL));
207+
208+ /* Specify type to allow for other types to be added in the future */
209+ var = NIH_MUST (nih_sprintf (NULL, "SOCKET_TYPE=%s", socket_type));
210+ NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
211+
212+ var = NIH_MUST (nih_sprintf (NULL, "SOCKET_VARIANT=%s",
213+ sock->sun_addr.sun_path[0] ? "named" : "abstract"));
214+ NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
215+
216+ var = NIH_MUST (nih_sprintf (NULL, "CLIENT_UID=%u", (unsigned int)client->ucred.uid));
217+ NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
218+
219+ var = NIH_MUST (nih_sprintf (NULL, "CLIENT_GID=%u", (unsigned int)client->ucred.gid));
220+ NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
221+
222+ var = NIH_MUST (nih_sprintf (NULL, "CLIENT_PID=%u", (unsigned int)client->ucred.pid));
223+ NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
224+
225+ var = NIH_MUST (nih_sprintf (NULL, "PATH=%s", socket_path));
226+ NIH_MUST (nih_str_array_addp (&env, NULL, NULL, var));
227+
228+ /* Add the name=value pair */
229+ NIH_MUST (nih_str_array_addn (&env, NULL, NULL, pair, len));
230+
231+ pending_call = upstart_emit_event (upstart,
232+ event_name, env, FALSE,
233+ NULL, emit_event_error, NULL,
234+ NIH_DBUS_TIMEOUT_NEVER);
235+
236+ if (! pending_call) {
237+ NihError *err;
238+ err = nih_error_get ();
239+ nih_warn ("%s", err->message);
240+ nih_free (err);
241+ }
242+
243+ dbus_pending_call_unref (pending_call);
244+}

Subscribers

People subscribed via source and target branches