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