[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#693702: tpu: weechat/0.3.8-2 (pre-approval)



Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: tpu

Hi,

I'd like to get your approval about the upload of weechat 0.3.8-2 to
testing-proposed-updates in order to fix 2 security issues:

 1) a remote attacker might crash weechat by forging malicious IRC
   messages, CVE-2012-5854, #693026
 2) a remote attacker could exploit the process handling API used by
   scripts to execute arbitrary commands, a CVE ID has been requested
   but not yet assigned

The first bug has been fixed in sid with weechat 0.3.9.1, the second
one has been fixed with the upload of weechat 0.3.9.2 a few hours ago

Attached is the diff.

Thanks for your replies.


Regards,


M.

-- System Information:
Debian Release: 6.0.6
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.32-5-xen-amd64 (SMP w/4 CPU cores)
Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash
diff -Nru weechat-0.3.8/debian/changelog weechat-0.3.8/debian/changelog
--- weechat-0.3.8/debian/changelog	2012-06-03 07:57:24.000000000 +0000
+++ weechat-0.3.8/debian/changelog	2012-11-19 13:14:43.000000000 +0000
@@ -1,3 +1,13 @@
+weechat (0.3.8-2) testing-proposed-updates; urgency=high
+
+  * Add a patch to fix a crash while decoding IRC colors in strings. A remote
+  attacker could exploit this issue by forging malicious IRC messages:
+  CVE-2012-5854 (Closes: #693026)
+  * Add a patch to not call shell to execute command in hook_process (fix
+    security issue when a plugin/script gives untrusted command)
+
+ -- Emmanuel Bouthenot <kolter@debian.org>  Mon, 19 Nov 2012 13:10:18 +0000
+
 weechat (0.3.8-1) unstable; urgency=low
 
   * New upstream release
diff -Nru weechat-0.3.8/debian/patches/fix_crash_with_irc_colors weechat-0.3.8/debian/patches/fix_crash_with_irc_colors
--- weechat-0.3.8/debian/patches/fix_crash_with_irc_colors	1970-01-01 00:00:00.000000000 +0000
+++ weechat-0.3.8/debian/patches/fix_crash_with_irc_colors	2012-11-12 12:30:49.000000000 +0000
@@ -0,0 +1,139 @@
+From: Sebastien Helleu <flashcode@flashtux.org>
+Description: fix crash when decoding IRC colors in strings
+Origin: upstream, http://git.savannah.gnu.org/gitweb/?p=weechat.git;a=commitdiff;h=80f477f2c37b46bafcde1a35660cf095a95a05c4
+Bug: http://savannah.nongnu.org/bugs/?37704
+Bug-Debian: http://bugs.debian.org/693026
+Forwarded: not-needed
+Last-Update: 2012-11-12
+--- a/src/plugins/irc/irc-color.c
++++ b/src/plugins/irc/irc-color.c
+@@ -62,13 +62,15 @@ char *irc_color_to_weechat[IRC_NUM_COLORS] =
+ char *
+ irc_color_decode (const char *string, int keep_colors)
+ {
+-    unsigned char *out, *ptr_string;
+-    int out_length, length, out_pos;
+-    char str_fg[3], str_bg[3], str_color[128], str_key[128];
++    unsigned char *out, *out2, *ptr_string;
++    int out_length, length, out_pos, length_to_add;
++    char str_fg[3], str_bg[3], str_color[128], str_key[128], str_to_add[128];
+     const char *remapped_color;
+     int fg, bg, bold, reverse, italic, underline, rc;
+ 
+     out_length = (strlen (string) * 2) + 1;
++    if (out_length < 128)
++        out_length = 128;
+     out = malloc (out_length);
+     if (!out)
+         return NULL;
+@@ -80,20 +82,27 @@ irc_color_decode (const char *string, int keep_colors)
+ 
+     ptr_string = (unsigned char *)string;
+     out[0] = '\0';
++    out_pos = 0;
+     while (ptr_string && ptr_string[0])
+     {
++        str_to_add[0] = '\0';
+         switch (ptr_string[0])
+         {
+             case IRC_COLOR_BOLD_CHAR:
+                 if (keep_colors)
+-                    strcat ((char *)out,
+-                            weechat_color((bold) ? "-bold" : "bold"));
++                {
++                    snprintf (str_to_add, sizeof (str_to_add), "%s",
++                              weechat_color ((bold) ? "-bold" : "bold"));
++                }
+                 bold ^= 1;
+                 ptr_string++;
+                 break;
+             case IRC_COLOR_RESET_CHAR:
+                 if (keep_colors)
+-                    strcat ((char *)out, weechat_color("reset"));
++                {
++                    snprintf (str_to_add, sizeof (str_to_add), "%s",
++                              weechat_color ("reset"));
++                }
+                 bold = 0;
+                 reverse = 0;
+                 italic = 0;
+@@ -106,22 +115,28 @@ irc_color_decode (const char *string, int keep_colors)
+             case IRC_COLOR_REVERSE_CHAR:
+             case IRC_COLOR_REVERSE2_CHAR:
+                 if (keep_colors)
+-                    strcat ((char *)out,
+-                            weechat_color((reverse) ? "-reverse" : "reverse"));
++                {
++                    snprintf (str_to_add, sizeof (str_to_add), "%s",
++                              weechat_color ((reverse) ? "-reverse" : "reverse"));
++                }
+                 reverse ^= 1;
+                 ptr_string++;
+                 break;
+             case IRC_COLOR_ITALIC_CHAR:
+                 if (keep_colors)
+-                    strcat ((char *)out,
+-                            weechat_color((italic) ? "-italic" : "italic"));
++                {
++                    snprintf (str_to_add, sizeof (str_to_add), "%s",
++                              weechat_color ((italic) ? "-italic" : "italic"));
++                }
+                 italic ^= 1;
+                 ptr_string++;
+                 break;
+             case IRC_COLOR_UNDERLINE_CHAR:
+                 if (keep_colors)
+-                    strcat ((char *)out,
+-                            weechat_color((underline) ? "-underline" : "underline"));
++                {
++                    snprintf (str_to_add, sizeof (str_to_add), "%s",
++                              weechat_color ((underline) ? "-underline" : "underline"));
++                }
+                 underline ^= 1;
+                 ptr_string++;
+                 break;
+@@ -194,22 +209,39 @@ irc_color_decode (const char *string, int keep_colors)
+                                       (bg >= 0) ? "," : "",
+                                       (bg >= 0) ? irc_color_to_weechat[bg] : "");
+                         }
+-                        strcat ((char *)out, weechat_color(str_color));
++                        snprintf (str_to_add, sizeof (str_to_add), "%s",
++                                  weechat_color (str_color));
+                     }
+                     else
+-                        strcat ((char *)out, weechat_color("resetcolor"));
++                    {
++                        snprintf (str_to_add, sizeof (str_to_add), "%s",
++                                  weechat_color ("resetcolor"));
++                    }
+                 }
+                 break;
+             default:
+                 length = weechat_utf8_char_size ((char *)ptr_string);
+                 if (length == 0)
+                     length = 1;
+-                out_pos = strlen ((char *)out);
+-                memcpy (out + out_pos, ptr_string, length);
+-                out[out_pos + length] = '\0';
++                memcpy (str_to_add, ptr_string, length);
++                str_to_add[length] = '\0';
+                 ptr_string += length;
+                 break;
+         }
++        if (str_to_add[0])
++        {
++            length_to_add = strlen (str_to_add);
++            if (out_pos + length_to_add >= out_length)
++            {
++                out_length *= 2;
++                out2 = realloc (out, out_length);
++                if (!out2)
++                    return (char *)out;
++                out = out2;
++            }
++            memcpy (out + out_pos, str_to_add, length_to_add + 1);
++            out_pos += length_to_add;
++        }
+     }
+ 
+     return (char *)out;
diff -Nru weechat-0.3.8/debian/patches/fix_hook_process weechat-0.3.8/debian/patches/fix_hook_process
--- weechat-0.3.8/debian/patches/fix_hook_process	1970-01-01 00:00:00.000000000 +0000
+++ weechat-0.3.8/debian/patches/fix_hook_process	2012-11-19 09:53:57.000000000 +0000
@@ -0,0 +1,255 @@
+From: Sebastien Helleu <flashcode@flashtux.org>
+Description: do not call shell to execute command in hook_process (fix security
+    issue when a plugin/script gives untrusted command)
+Forwarded: not-needed
+Last-Update: 2012-11-19
+--- a/src/core/wee-hook.c
++++ b/src/core/wee-hook.c
+@@ -1386,9 +1386,9 @@
+ void
+ hook_process_child (struct t_hook *hook_process)
+ {
+-    char *exec_args[4] = { "sh", "-c", NULL, NULL };
++    char **exec_args;
+     const char *ptr_url;
+-    int rc;
++    int rc, i;
+ 
+     /*
+      * close stdin, so that process will fail to read stdin (process reading
+@@ -1427,10 +1427,24 @@
+     else
+     {
+         /* launch command */
+-        exec_args[2] = HOOK_PROCESS(hook_process, command);
+-        execvp (exec_args[0], exec_args);
++        exec_args = string_split_shell (HOOK_PROCESS(hook_process, command));
++        if (exec_args)
++        {
++            if (weechat_debug_core >= 1)
++            {
++                log_printf ("hook_process, command='%s'",
++                            HOOK_PROCESS(hook_process, command));
++                for (i = 0; exec_args[i]; i++)
++                {
++                    log_printf ("  args[%02d] == '%s'", i, exec_args[i]);
++                }
++            }
++            execvp (exec_args[0], exec_args);
++        }
+ 
+         /* should not be executed if execvp was ok */
++        if (exec_args)
++            string_free_split (exec_args);
+         fprintf (stderr, "Error with command '%s'\n",
+                  HOOK_PROCESS(hook_process, command));
+         rc = EXIT_FAILURE;
+--- a/src/core/wee-string.c
++++ b/src/core/wee-string.c
+@@ -1139,6 +1139,196 @@
+ }
+ 
+ /*
++ * string_split_shell: split a string like the shell does for a command with
++ *                     arguments.
++ *                     Note: result must be freed with string_free_split.
++ *                     This function is a C conversion of python class "shlex"
++ *                     (file: Lib/shlex.py in python repository)
++ *                     Doc: http://docs.python.org/3/library/shlex.html
++ *                     Copyrights in shlex.py:
++ *                       Module and documentation by Eric S. Raymond, 21 Dec 1998
++ *                       Input stacking and error message cleanup added by ESR, March 2000
++ *                       push_source() and pop_source() made explicit by ESR, January 2001.
++ *                       Posix compliance, split(), string arguments, and
++ *                       iterator interface by Gustavo Niemeyer, April 2003.
++ */
++
++char **
++string_split_shell (const char *string)
++{
++    int temp_len, num_args, add_char_to_temp, add_temp_to_args, quoted;
++    char *string2, *temp, **args, **args2, state, escapedstate;
++    char *ptr_string, *ptr_next, saved_char;
++
++    if (!string)
++        return NULL;
++
++    string2 = strdup (string);
++    if (!string2)
++        return NULL;
++
++    /*
++     * prepare "args" with one pointer to NULL, the "args" will be reallocated
++     * later, each time a new argument is added
++     */
++    num_args = 0;
++    args = malloc ((num_args + 1) * sizeof (args[0]));
++    if (!args)
++    {
++        free (string2);
++        return NULL;
++    }
++    args[0] = NULL;
++
++    /* prepare a temp string for working (adding chars one by one) */
++    temp = malloc ((2 * strlen (string)) + 1);
++    if (!temp)
++    {
++        free (string2);
++        free (args);
++        return NULL;
++    }
++    temp[0] = '\0';
++    temp_len = 0;
++
++    state = ' ';
++    escapedstate = ' ';
++    quoted = 0;
++    ptr_string = string2;
++    while (ptr_string[0])
++    {
++        add_char_to_temp = 0;
++        add_temp_to_args = 0;
++        ptr_next = utf8_next_char (ptr_string);
++        saved_char = ptr_next[0];
++        ptr_next[0] = '\0';
++        if (state == ' ')
++        {
++            if ((ptr_string[0] == ' ') || (ptr_string[0] == '\t')
++                || (ptr_string[0] == '\r') || (ptr_string[0] == '\n'))
++            {
++                if (temp[0] || quoted)
++                    add_temp_to_args = 1;
++            }
++            else if (ptr_string[0] == '\\')
++            {
++                escapedstate = 'a';
++                state = ptr_string[0];
++            }
++            else if ((ptr_string[0] == '\'') || (ptr_string[0] == '"'))
++            {
++                state = ptr_string[0];
++            }
++            else
++            {
++                add_char_to_temp = 1;
++                state = 'a';
++            }
++        }
++        else if ((state == '\'') || (state == '"'))
++        {
++            quoted = 1;
++            if (ptr_string[0] == state)
++            {
++                state = 'a';
++            }
++            else if ((state == '"') && (ptr_string[0] == '\\'))
++            {
++                escapedstate = state;
++                state = ptr_string[0];
++            }
++            else
++            {
++                add_char_to_temp = 1;
++            }
++        }
++        else if (state == '\\')
++        {
++            if (((escapedstate == '\'') || (escapedstate == '"'))
++                && (ptr_string[0] != state) && (ptr_string[0] != escapedstate))
++            {
++                temp[temp_len] = state;
++                temp_len++;
++                temp[temp_len] = '\0';
++            }
++            add_char_to_temp = 1;
++            state = escapedstate;
++        }
++        else if (state == 'a')
++        {
++            if ((ptr_string[0] == ' ') || (ptr_string[0] == '\t')
++                || (ptr_string[0] == '\r') || (ptr_string[0] == '\n'))
++            {
++                state = ' ';
++                if (temp[0] || quoted)
++                    add_temp_to_args = 1;
++            }
++            else if (ptr_string[0] == '\\')
++            {
++                escapedstate = 'a';
++                state = ptr_string[0];
++            }
++            else if ((ptr_string[0] == '\'') || (ptr_string[0] == '"'))
++            {
++                state = ptr_string[0];
++            }
++            else
++            {
++                add_char_to_temp = 1;
++            }
++        }
++        if (add_char_to_temp)
++        {
++            memcpy (temp + temp_len, ptr_string, ptr_next - ptr_string);
++            temp_len += (ptr_next - ptr_string);
++            temp[temp_len] = '\0';
++        }
++        if (add_temp_to_args)
++        {
++            num_args++;
++            args2 = realloc (args, (num_args + 1) * sizeof (args[0]));
++            if (!args2)
++            {
++                free (string2);
++                free (temp);
++                return args;
++            }
++            args = args2;
++            args[num_args - 1] = strdup (temp);
++            args[num_args] = NULL;
++            temp[0] = '\0';
++            temp_len = 0;
++            escapedstate = ' ';
++            quoted = 0;
++        }
++        ptr_next[0] = saved_char;
++        ptr_string = ptr_next;
++    }
++
++    if (temp[0] || (state != ' '))
++    {
++        num_args++;
++        args2 = realloc (args, (num_args + 1) * sizeof (args[0]));
++        if (!args2)
++        {
++            free (string2);
++            free (temp);
++            return args;
++        }
++        args = args2;
++        args[num_args - 1] = strdup (temp);
++        args[num_args] = NULL;
++        temp[0] = '\0';
++        temp_len = 0;
++    }
++
++    free (string2);
++    free (temp);
++
++    return args;
++}
++
++/*
+  * string_free_split: free a split string
+  */
+ 
+--- a/src/core/wee-string.h
++++ b/src/core/wee-string.h
+@@ -59,6 +59,7 @@
+ extern int string_has_highlight_regex (const char *string, const char *regex);
+ extern char **string_split (const char *string, const char *separators,
+                             int keep_eol, int num_items_max, int *num_items);
++extern char **string_split_shell (const char *string);
+ extern void string_free_split (char **split_string);
+ extern char *string_build_with_split_string (const char **split_string,
+                                              const char *separator);
diff -Nru weechat-0.3.8/debian/patches/series weechat-0.3.8/debian/patches/series
--- weechat-0.3.8/debian/patches/series	1970-01-01 00:00:00.000000000 +0000
+++ weechat-0.3.8/debian/patches/series	2012-11-19 09:47:51.000000000 +0000
@@ -0,0 +1,2 @@
+fix_crash_with_irc_colors
+fix_hook_process

Reply to: