From: Alexander Nozdrin
Date: November 23 2010 9:22am
Subject: Re: bzr commit into mysql-5.1-bugteam branch (mats.kindahl:3508)
 Bug#58246
List-Archive: http://lists.mysql.com/commits/124709
Message-Id: <4CEB87E5.7000800@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi Mats,

thank you for the patch. The fix itself is understandable and correct.

However, I'm thinking about the following things:

   - how about adding a test case?

   - could you please add a comment similar to the one in sql_udf.cc
     (about Windows-specificity)?

   - while we're at it, could you please grep for FN_LIBCHAR and make
     sure all other places are covered properly? I glanced over the "grep
     results" and few places looked suspicious to me...

   - may be it's time we introduce a new function to look for "path
     delimiters", and use it here, there and elsewhere (at least
     in sql_udf.cc and in sql_pluginc.cc)?

Thank you!

On 22.11.2010 18:10, Mats Kindahl wrote:
> #At file:///home/bzr/bugs/b58246-5.1-bugteam/ based on revid:davi.arnaut@stripped
>
>   3508 Mats Kindahl	2010-11-22
>        BUG#58246: INSTALL PLUGIN not secure&  crashable
>
>        When installing plugins, there is a missing check
>        for slash (/) in the path on Windows. Note that on
>        Windows, both / and \ can be used to separate
>        directories.
>
>        This patch fixes the issue by adding a check for /
>        on Windows, similar to how it is done in sql_udf.cc.
>
>      modified:
>        sql/sql_plugin.cc
> === modified file 'sql/sql_plugin.cc'
> --- a/sql/sql_plugin.cc	2010-08-05 12:10:24 +0000
> +++ b/sql/sql_plugin.cc	2010-11-22 15:09:51 +0000
> @@ -361,6 +361,7 @@ static st_plugin_dl *plugin_dl_add(const
>       plugin directory are used (to make this even remotely secure).
>     */
>     if (my_strchr(files_charset_info, dl->str, dl->str + dl->length, FN_LIBCHAR) ||
> +      IF_WIN(my_strchr(files_charset_info, dl->str, dl->str + dl->length, '/'), 0) ||
>         check_string_char_length((LEX_STRING *) dl, "", NAME_CHAR_LEN,
>                                  system_charset_info, 1) ||
>         plugin_dir_len + dl->length + 1>= FN_REFLEN)
>
>
>
>
>