Security problems in gkrellm-newsticker

[Martin Schulze]

Brian Campbell discovered two security-related problems in
gkrellm-newsticker, a plugin for the gkrellm system monitor program,
which provides a news ticker from RDF feeds. The following IDs were
assigned:

CAN-2003-0205

gkrellm-newsticker can launch a web browser of the user's choice
when the ticker title is clicked by using the URI given by the feed.
However, special shell characters are not properly escaped enabling
a malicious feed to execute arbitrary shell commands on the clients
machine.

CAN-2003-0206

gkrellm-newsticker crashes the entire gkrellm system on RDF files
where link or title elements are not entirely on a single line. A
malicious server could therefore craft a denial of service. The
nature of the crash means that it cannot be exploited to perform any
other actions (it simply attempts to allocate a silly amount of
memory).

Below is a patch from Brian Campbell to fix both problems.

As the parser does not make any real attempt to parse XML, the patch
just takes the remainder of the first line.


diff -ur gkrellm-newsticker-0.3.orig/newsticker.c gkrellm-newsticker-0.3/newsticker.c
--- gkrellm-newsticker-0.3.orig/newsticker.c Sun Jan 20 21:02:40 2002
+++ gkrellm-newsticker-0.3/newsticker.c Sat Apr 5 09:37:18 2003
@@ -292,7 +292,12 @@
pt = strchr(pt, '>');
pt++;
pt2 = strstr(buf, "</link>");
- nt->link = g_strndup(pt, (pt2 - pt));
+ /* Can't handle multiple lines properly, but at least make some
+ * effort. */
+ if (pt2)
+ nt->link = g_strndup(pt, (pt2 - pt));
+ else
+ nt->link = g_strdup(pt);
flag++;
continue;
}
@@ -306,10 +311,20 @@
pt = strchr(pt, '>');
pt++;
pt2 = strstr(buf, "</title>");
- if (flag == 2)
- nt->headline = g_strndup(pt, (pt2 - pt));
- else
- nt->headline = g_strconcat(nt->headline, " --- ", g_strndup(pt, (pt2 - pt)), NULL);
+ /* Again, let's not fail completely when the element spans more
+ * than one line. */
+ if (pt2)
+ {
+ if (flag == 2)
+ nt->headline = g_strndup(pt, (pt2 - pt));
+ else
+ nt->headline = g_strconcat(nt->headline, " --- ", g_strndup(pt, (pt2 - pt)), NULL);
+ } else {
+ if (flag == 2)
+ nt->headline = g_strdup(pt);
+ else
+ nt->headline = g_strconcat(nt->headline, " --- ", g_strdup(pt), NULL);
+ }
flag++;
if (flag > (num_headlines+1))
break;
@@ -474,10 +489,36 @@
return FALSE;
}

+/* Make a URI suitable for use in a shell command. */
+static gchar *escape_uri(gchar *uri)
+{
+ gchar *cur, *result, *resultcur;
+ int count = 1;
+
+ for (cur = uri; *cur; cur++)
+ count += (*cur == '\'') ? 3 : 1;
+
+ result = g_malloc(count);
+ for (cur = uri, resultcur = result; *cur; cur++)
+ {
+ if (*cur == '\'')
+ {
+ *resultcur++ = '%';
+ *resultcur++ = '2';
+ *resultcur++ = '7';
+ }
+ else
+ *resultcur++ = *cur;
+ }
+ *resultcur = '\0';
+
+ return result;
+}

static gint panel_click_event(GtkWidget *widget, GdkEventButton *ev)
{
gchar *command;
+ gchar *link;
GList *list;
Newsticker *nt;

@@ -490,7 +531,9 @@
{
if ((ev->button == 1) && (strcmp(nt->link, "NULL")))
{
- command = g_strdup_printf(browser, nt->link);
+ link = escape_uri(nt->link);
+ command = g_strdup_printf(browser, link);
+ g_free(link);
command = g_strconcat(command, " &", NULL);
system(command);
g_free(command);


Regards,

Joey
Debian Security Team

--
The good thing about standards is that there are so many to choose from.
-- Andrew S. Tanenbaum