justinhunt / moodle-filter_poodll

The PoodLL Filter
6 stars 17 forks source link

unsafe variable value setting #4

Closed ndunand closed 9 years ago

ndunand commented 9 years ago

in https://github.com/justinhunt/moodle-filter_poodll/blob/master/poodllresourcelib.php#L3434 , raw values are set for params, which does not work in French, as

filter_poodll.php:$string['recui_echo'] = 'Suppression d\'écho';

contains an apostrophe, which is considered a string delimiter in JS. Therefore, running the code as-is results in a JS error, and no recorder is displayer.

I've replaced $value by urlencode($value) in the above mentioned place in the code, and it seems to fix the problem.

justinhunt commented 9 years ago

Thanks for the report. And you are right it should be surrounded by urlencode() when it is used in a url.

In function filter_poodll_fetch_recorder_strings() https://github.com/justinhunt/moodle-filter_poodll/blob/master/poodllresourcelib.php#L3040 I use urlencode. I thought that was the only place I used those lang strings.

I believe that you did have this problem though. I am wondering if you could tell me where it occurred?

Justin

On Tue, Aug 18, 2015 at 11:40 PM, Nicolas Dunand notifications@github.com wrote:

in https://github.com/justinhunt/moodle-filter_poodll/blob/master/poodllresourcelib.php#L3434 , raw values are set for params, which does not work in French, as

filter_poodll.php:$string['recui_echo'] = 'Suppression d\'écho';

which contains an apostrophe, which is considered a string delimiter in JS. Therefore, running the code as-is results in a JS error, and no recorder is displayer.

I've replaced $value by urlencode($value) in the above mentioned place in the code, and it seems to fix the problem.

— Reply to this email directly or view it on GitHub https://github.com/justinhunt/moodle-filter_poodll/issues/4.

ndunand commented 9 years ago

You are right. My bad for using an outdated version of the plugin. Sorry for the bother.

justinhunt commented 9 years ago

No problem it all. I appreciate you looking at it and letting me know. The exceptionally odd thing is that yesterday I also received a report of the the same mistake on another plugin. And it was a similar accented French string that broke it. In that case it was a bug I had to fix.

On Wed, Aug 19, 2015 at 3:38 PM, Nicolas Dunand notifications@github.com wrote:

Closed #4 https://github.com/justinhunt/moodle-filter_poodll/issues/4.

— Reply to this email directly or view it on GitHub https://github.com/justinhunt/moodle-filter_poodll/issues/4#event-386144375 .