Bugtraq: Security problems in gkrellm-newsticker

[Martin Schulze <joey () infodrom org>]

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