Re: [Tiki-devel] Preference setting and cross-site request forgery

[Patrick P. <pa...@sy...>]

Thanks jonny.

I’ve made the changes in the following commits:
https://sourceforge.net/p/tikiwiki/code/56119 <https://sourceforge.net/p/tikiwiki/code/56119>
https://sourceforge.net/p/tikiwiki/code/56124/ <https://sourceforge.net/p/tikiwiki/code/56124/>

This allows us to have the include_prefpage.php in addons/addon_name/admin/ folder and also makes the php file optional if there’s no additional work that needs to be done.

Feedback welcome!

Thanks,
Pat


> On Sep 2, 2015, at 7:22 AM, Jonny Bradley <jo...@ti...> wrote:
> 
> Hi Pat
> 
> That change looks fine to me, the check for the file was only for the include i think, so issuing the ticket should be find presuming the add-on file gets included somewhere else somehow (shouldn't that be there too?)
> 
> My 2¢
> 
> jb
> 
> 
> 
>> On 1 Sep 2015, at 15:54, Patrick Proulx <pa...@sy... <mailto:pa...@sy...>> wrote:
>> 
>> Hey guys,
>> 
>> I’m currently working in the Addons feature and ran into some CSRF issues when trying to set prefs. I’ve figured out the issue but had a follow-up question for you guys. 
>> 
>> The issue appears to be around line 529 of tiki-admin.php. It checks if an associated admin/include_$pagename.php exists before generating the key/ticket to change the pref (this is an issue from the Addon’s perspective since it doesn’t check the addon subfolder - this is something I’ll be adding).
>> 
>> if (isset($_REQUEST['page'])) {
>>    $adminPage = $_REQUEST['page'];
>>    if (file_exists("admin/include_$adminPage.php")) {
>>       $check = key_get(null, null, null, false);
>>       $smarty->assign('ticket', $check['ticket']);
>>       include_once ("admin/include_$adminPage.php");
>>       $url = 'tiki-admin.php' . '?page=' . $adminPage;
>>    }
>> 
>> From what I can tell the ticket used to be generated in that .php file but now appears to be generically generated in the key_get() function on line 530 instead.
>> So my question is whether it’s necessary to have the associated include_$pagename.php file to create the ticket or if this should be made optional. 
>> 
>> if (isset($_REQUEST['page'])) {
>>    $adminPage = $_REQUEST['page'];
>>    $check = key_get(null, null, null, false);
>>    $smarty->assign('ticket', $check['ticket']);
>>    if (file_exists("admin/include_$adminPage.php")) {
>>       include_once ("admin/include_$adminPage.php");
>>    }
>>    $url = 'tiki-admin.php' . '?page=' . $adminPage;
>> 
>> Please let me know if I’m missing something and not properly understanding the purpose of the include_*.php files. 
>> 
>> Thank you!
>> 
>> Pat
>> ------------------------------------------------------------------------------
>> _______________________________________________
>> TikiWiki-devel mailing list
>> Tik...@li... <mailto:Tik...@li...>
>> https://lists.sourceforge.net/lists/listinfo/tikiwiki-devel
> 
> ------------------------------------------------------------------------------
> Monitor Your Dynamic Infrastructure at Any Scale With Datadog!
> Get real-time metrics from all of your servers, apps and tools
> in one place.
> SourceForge users - Click here to start your Free Trial of Datadog now!
> http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140_______________________________________________
> TikiWiki-devel mailing list
> Tik...@li...
> https://lists.sourceforge.net/lists/listinfo/tikiwiki-devel