List:Commits« Previous MessageNext Message »
From:Mats Kindahl Date:November 24 2010 5:45pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (mats.kindahl:3508)
Bug#58246
View as plain text  
On 11/23/2010 10:22 AM, Alexander Nozdrin wrote:
> 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?

Added.

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

I actually removed the comment from the other place since it calls a
dedicated function for checking if it is a path.

>
>   - 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...

I did, and there are indeed some places that look suspicious. They are
not related to this bug, but they seem to use strrchr to fetch the
basename of the path. I'm going to check with the involved and possibly
report bugs for those cases when I have figured out if and/or how they
can be triggered.

>   - 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)?

I introduced charset-aware functions my_strspn and my_strcspn and
defined a check_if_path function using my_strcspn.

The patch is committed and ready for review.

Just my few cents,
Mats Kindahl

>
> 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)
>>
>>
>>
>>
>>
>

Thread
bzr commit into mysql-5.1-bugteam branch (mats.kindahl:3508) Bug#58246Mats Kindahl22 Nov
Re: bzr commit into mysql-5.1-bugteam branch (mats.kindahl:3508)Bug#58246Alexander Nozdrin23 Nov
  • Re: bzr commit into mysql-5.1-bugteam branch (mats.kindahl:3508)Bug#58246Mats Kindahl23 Nov
  • Re: bzr commit into mysql-5.1-bugteam branch (mats.kindahl:3508)Bug#58246Mats Kindahl24 Nov