Joomla and the use of Eval

621 views
Skip to first unread message

Thierry bela nanga

unread,
Jun 20, 2012, 10:38:25 AM6/20/12
to joomla-...@googlegroups.com

Hello all,

I have installed Joomla on a new server and I'm getting this fatal error

Fatal error: SUHOSIN - Use of eval is forbidden by configuration in /xxx/libraries/joomla/document/html/html.php(2) : eval()'d code on line xxx

maybe create_function should be used instead of eval,

Cheers,

--
http://tbela99.blogspot.com/

fax : (+33) 08 26 51 94 51

Sam Moffatt

unread,
Jun 20, 2012, 1:24:40 PM6/20/12
to joomla-...@googlegroups.com
We should probably see if we can eliminate eval all together however
replacing eval with create_function is probably not a good idea as
create_function has significant performance issues on all but the
latest versions of PHP.

Cheers,

Sam Moffatt
http://pasamio.id.au
> --
> You received this message because you are subscribed to the Google Groups
> "Joomla! CMS Development" group.
> To post to this group, send an email to joomla-...@googlegroups.com.
> To unsubscribe from this group, send email to
> joomla-dev-cm...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/joomla-dev-cms?hl=en-GB.

garyamort

unread,
Jun 20, 2012, 3:48:20 PM6/20/12
to joomla-...@googlegroups.com


On Wednesday, June 20, 2012 1:24:40 PM UTC-4, Samuel Moffatt wrote:
We should probably see if we can eliminate eval all together however
replacing eval with create_function is probably not a good idea as
create_function has significant performance issues on all but the
latest versions of PHP.

In this case, I'm not sure why eval is there to begin with.  The relevant lines are:
$str = 'return ' . implode(' ', $words) . ';';
return eval($str); 

$words is set using a helper, but drilling down I don't see any way PHP code could be added into the $words array, so there is no need to call return twice here, just eliminate the eval and return the string.

I went ahead and patched and submitted a pull request on it.....might as well see if my ide is acting properly now or if I'm still having tab issues.

Sam Moffatt

unread,
Jun 20, 2012, 6:03:01 PM6/20/12
to joomla-...@googlegroups.com
It's not PHP code, it's arithmetic code. It's allowing you to do stuff
like "left + leftnav".

See:
http://docs.joomla.org/Counting_modules_in_multiple_module_positions

Cheers,

Sam Moffatt
http://pasamio.id.au


> --
> You received this message because you are subscribed to the Google Groups
> "Joomla! CMS Development" group.
> To view this discussion on the web, visit
> https://groups.google.com/d/msg/joomla-dev-cms/-/7CzUdhZE524J.

Marius van Rijnsoever

unread,
Jun 20, 2012, 6:37:39 PM6/20/12
to joomla-...@googlegroups.com
What Gary meant was that the statement:

$str = 'return ' . implode(' ', $words) . ';';
return eval($str);

The words array always contains a numeric value:
$words[$i] = ((isset(parent::$_buffer['modules'][$name])) &&
(parent::$_buffer['modules'][$name] === false))
? 0
: count(JModuleHelper::getModules($name));

Therefore eval should be avoided (as $words can not contain PHP code)
and should be replaced with:
return implode(' ', $words);

Unless we are missing something here.

Thanks, Marius

Rouven Weßling

unread,
Jun 20, 2012, 6:43:23 PM6/20/12
to joomla-...@googlegroups.com
$words can not only contain numbers but also expressions. This happens when you use countModules() with an expression like $this->countModules( 'position-1 + position-2 - position-3' );

Rouven

garyamort

unread,
Jun 20, 2012, 6:46:16 PM6/20/12
to joomla-...@googlegroups.com
Yes, I was missing the fact that the words array is not just numbers, it also has conditions.

Ie LeftNav or RightNav

$words[0]='LeftNav' 
$words[1]='or'
$words[2]='RightNav'

The for loop will then turn that into:
$words[0]='1' 
$words[1]='or'
$words[2]='0'

So then the implode turns all those counts back into one giant condition:
1or0

And eval then functions on it: 1or0 is true

I rewrote and resubmitted my pull request.  I can see WHY eval was used, it's an obscene amount of switch statements needed to process that in php and since the computer can already process it for us[using eval] it's hard to get motivated to fix it.

Nevertheless, avoiding eval is preferable so I went ahead and cut and paste and paste and paste
>> To post to this group, send an email to joomla-dev-cms@googlegroups.com.
>> To unsubscribe from this group, send email to
>> For more options, visit this group at
>> http://groups.google.com/group/joomla-dev-cms?hl=en-GB.
>
> --
> You received this message because you are subscribed to the Google Groups "Joomla! CMS Development" group.
> To post to this group, send an email to joomla-dev-cms@googlegroups.com.
> To unsubscribe from this group, send email to joomla-dev-cms+unsubscribe@googlegroups.com.

Marius van Rijnsoever

unread,
Jun 20, 2012, 6:52:55 PM6/20/12
to joomla-...@googlegroups.com
eval is also used in the updater:
https://github.com/search?q=eval+repo%3Ajoomla%2Fjoomla-cms&repo=&langOverride=&start_value=1&type=Code&language=
>> >> joomla-...@googlegroups.com.
>> >> To unsubscribe from this group, send email to
>> >> joomla-dev-cm...@googlegroups.com.
>> >> For more options, visit this group at
>> >> http://groups.google.com/group/joomla-dev-cms?hl=en-GB.
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> > Groups "Joomla! CMS Development" group.
>> > To post to this group, send an email to joomla-...@googlegroups.com.
>> > To unsubscribe from this group, send email to
>> > joomla-dev-cm...@googlegroups.com.
>> > For more options, visit this group at
>> > http://groups.google.com/group/joomla-dev-cms?hl=en-GB.
>> >
>
> --
> You received this message because you are subscribed to the Google Groups
> "Joomla! CMS Development" group.
> To view this discussion on the web, visit
> https://groups.google.com/d/msg/joomla-dev-cms/-/vs5TVzDxJxAJ.
>
> To post to this group, send an email to joomla-...@googlegroups.com.
> To unsubscribe from this group, send email to
> joomla-dev-cm...@googlegroups.com.

Rouven Weßling

unread,
Jun 20, 2012, 6:55:28 PM6/20/12
to joomla-...@googlegroups.com
I had that one on the radar. I think it can be safely removed, we should fix the strict error caused by that code too.

Best regards
Rouven

garyamort

unread,
Jun 20, 2012, 10:34:56 PM6/20/12
to joomla-...@googlegroups.com
Is there any need for this functionality?  The entire block could be removed if countModules only counts modules in a single position.

atomic, for example, uses: 

<?php if($this->countModules('atomic-topquote') or $this->countModules('position-15') ) : ?>

That would work fine if the entire block of code was shortend to only return one position's count.
>>> To post to this group, send an email to joomla-dev-cms@googlegroups.com.
>>> To unsubscribe from this group, send email to
>>> For more options, visit this group at
>>> http://groups.google.com/group/joomla-dev-cms?hl=en-GB.
>>
>> --
>> You received this message because you are subscribed to the Google Groups "Joomla! CMS Development" group.
>> To post to this group, send an email to joomla-dev-cms@googlegroups.com.
>> To unsubscribe from this group, send email to joomla-dev-cms+unsubscribe@googlegroups.com.
>> For more options, visit this group at http://groups.google.com/group/joomla-dev-cms?hl=en-GB.
>>
>
> --
> You received this message because you are subscribed to the Google Groups "Joomla! CMS Development" group.
> To post to this group, send an email to joomla-dev-cms@googlegroups.com.
> To unsubscribe from this group, send email to joomla-dev-cms+unsubscribe@googlegroups.com.

elin

unread,
Jun 20, 2012, 10:47:40 PM6/20/12
to joomla-...@googlegroups.com
Yes you often want to count multiple positions when making a collapse decision.

Elin
>>>>> To unsubscribe from this group, send email to
>>>>> For more options, visit this group at
>>>>> http://groups.google.com/group/joomla-dev-cms?hl=en-GB.
>>>>
>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups "Joomla! CMS Development" group.
>>>> To post to this group, send an email to joomla-dev-cms@googlegroups.com.
>>>> To unsubscribe from this group, send email to
>>>> For more options, visit this group at
>>>> http://groups.google.com/group/joomla-dev-cms?hl=en-GB.
>>>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Joomla! CMS Development" group.
>> To view this discussion on the web, visit
>> https://groups.google.com/d/msg/joomla-dev-cms/-/vs5TVzDxJxAJ.
>>
>> To post to this group, send an email to joomla-dev-cms@googlegroups.com.
>> To unsubscribe from this group, send email to
>> For more options, visit this group at
>> http://groups.google.com/group/joomla-dev-cms?hl=en-GB.
>
> --
> You received this message because you are subscribed to the Google Groups "Joomla! CMS Development" group.
> To post to this group, send an email to joomla-dev-cms@googlegroups.com.
> To unsubscribe from this group, send email to joomla-dev-cms+unsubscribe@googlegroups.com.

Thierry bela nanga

unread,
Jun 21, 2012, 4:59:02 AM6/21/12
to joomla-...@googlegroups.com
if we can't use eval,

I think we can work with create_function while looking for a better solution. I'll try to fix it that way,

Cheers,

To view this discussion on the web, visit https://groups.google.com/d/msg/joomla-dev-cms/-/HovsxKAyUtwJ.

To post to this group, send an email to joomla-...@googlegroups.com.
To unsubscribe from this group, send email to joomla-dev-cm...@googlegroups.com.

For more options, visit this group at http://groups.google.com/group/joomla-dev-cms?hl=en-GB.



--

garyamort

unread,
Jun 21, 2012, 9:56:21 AM6/21/12
to joomla-...@googlegroups.com


On Wednesday, June 20, 2012 10:47:40 PM UTC-4, elin wrote:
Yes you often want to count multiple positions when making a collapse decision.



Sure, but I don't know if most templates even use this function.
Atomic for example does this:
<?php if($this->countModules('atomic-topquote') or $this->countModules('position-15') ) : ?> 

Which will work without needing to call eval because each countModules method call is only counting modules in a /single/ position.

If it was using the eval of countModules then it would have been:
<?php if($this->countModules('atomic-topquote or position-15') ) : ?> 

So if most templates do the comparisons themselves in PHP, instead of using countModules to do the comparison, then all that logic can be removed.

And a complete tangent, at the very least there should be checks on the parameters to test for 1 or 2 elements in the call and handle them without calling eval.
IE:
$this->countModules('atomic-topquote')  
One parameter, so there is no need to call eval, don't slow down the system by calling it.

And
$this->countModules('atomic-topquote or')
Two parameters, I suppose return false/zero and flag an exception because this shouldn't happen......erg, and I just realized that by not checking for only 2 params this is minor security hole....ick...will go and report it.
   
I'm not a die-hard anti-eval advocate.  I think eval has it's place, but it's place should be the last option when there is no other way to allow for PHP to be added.  I just don't think the core code is the place to have eval.  



 

garyamort

unread,
Jun 21, 2012, 11:14:02 AM6/21/12
to joomla-...@googlegroups.com
create function doesn't really solve the issue.   The problem is that you can't have variable operators.
ie 
$a = 0; $b='+'; $c='1';
$a$$b$c is not the result of 0+1
$a$b$c is also not the result of 0+1
$a.$b.$c is the string '0+1' but not the result of the operation, ie 1

The purpose/goal is to be able to process something like:
topNav or bottomNav and leftSidebar or rightSideBar + contentAside

A simple for loop combined with a switch can loop through it from left to right, accumulating the result ie:
result = topNav or bottomNav
result = result and leftSidebar
result = result and rightSideBar
result = result + contentAside

This won't work though because it breaks the current documented spec for operator precedence:
The correct answer is supposed to be:
Array: topNav or bottomNav and leftSidebar or rightSideBar + contentAside
result1 = rightSideBar + contentAside
Array: topNav or bottomNav and leftSidebar or result1
result2 = bottomNav and leftSidebar
Array: topNav or result2 or result1
result3 = topNav or result2
Array: result3 or result1
result4 = result3 or result1
finalresult = result4.

In order to keep to the documentation, I had to create 7 switch statements in order to manipulate the array and process the operators in the correct sequence.  I think some code duplication could be reduced by checking the various Joomla variable processing functions as there should already be a few static methods like JInput::add($var1, $var2) but I don't know the full api well enough to know where those are...plus it means a patchfile which affects a number of files as the operators which are missing static methods will need to be added.

Thierry bela nanga

unread,
Jun 21, 2012, 11:31:49 AM6/21/12
to joomla-...@googlegroups.com
not sure about that. what I'll do is

$fn = create_function('','return ' . implode(' ', $words) . ';');

return $fn();

I have not tested, but I'm pretty sure it will work fine.

--
You received this message because you are subscribed to the Google Groups "Joomla! CMS Development" group.
To view this discussion on the web, visit https://groups.google.com/d/msg/joomla-dev-cms/-/-J2bk0YR4i0J.

To post to this group, send an email to joomla-...@googlegroups.com.
To unsubscribe from this group, send email to joomla-dev-cm...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/joomla-dev-cms?hl=en-GB.

Rouven Weßling

unread,
Jun 21, 2012, 2:20:16 PM6/21/12
to joomla-...@googlegroups.com
create_function isn't much better than eval, many hosts disable that one as well.

The check for only one parameter would be nice thou, that should eliminate the need for eval on most sites.

Rouven

Chris Davenport

unread,
Jun 21, 2012, 3:02:12 PM6/21/12
to joomla-...@googlegroups.com
Whilst it would be possible to shift the expression evaluation out of countModules and put in the template PHP code, this would be an API change that would break backwards compatibility for all templates that make use of it.  Although I would be quite comfortable making such a change, I think that many, many template "users" are not PHP coders and might well struggle.

My preferred approach would be to write some PHP code to exactly replace the functionality currently provided by the eval call.  The behaviour of countModules is well-specified here: http://docs.joomla.org/Operators_for_use_with_the_countModules_function so a first step would be to write unit tests that cover that specification.  Then you will have the tests available to ensure that whatever new code is used to replace the eval, it will be 100% backwards compatible and templates would not need to be changed.

Chris.


--
You received this message because you are subscribed to the Google Groups "Joomla! CMS Development" group.
To post to this group, send an email to joomla-...@googlegroups.com.
To unsubscribe from this group, send email to joomla-dev-cm...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/joomla-dev-cms?hl=en-GB.




--
Chris Davenport
Joomla Production Leadership Team

garyamort

unread,
Jun 21, 2012, 6:43:43 PM6/21/12
to joomla-...@googlegroups.com
See  https://github.com/joomla/joomla-platform/pull/1296#issuecomment-6481977 

Though I recommend putting that pull request on hold for a few days since I also submitted a security patch for countModules.  If the security patch goes in, then there are fewer issues to test for.  I can hopefully revisit it and add unit tests for countModules in a serperate pull request to go through first, and then resubmit this one.

Note: my patch to 'replicate' is also an API change, albeit an undocumented api change.

currently countModules('position1 and position2') will yield the same result as countModules('position1 && position2') because no checking of the operands is done to ensure their in the documented set.  My patch ONLY replicates functionality as far as the documented api goes.



On Thursday, June 21, 2012 3:02:12 PM UTC-4, Chris Davenport wrote:
Whilst it would be possible to shift the expression evaluation out of countModules and put in the template PHP code, this would be an API change that would break backwards compatibility for all templates that make use of it.  Although I would be quite comfortable making such a change, I think that many, many template "users" are not PHP coders and might well struggle.

My preferred approach would be to write some PHP code to exactly replace the functionality currently provided by the eval call.  The behaviour of countModules is well-specified here: http://docs.joomla.org/Operators_for_use_with_the_countModules_function so a first step would be to write unit tests that cover that specification.  Then you will have the tests available to ensure that whatever new code is used to replace the eval, it will be 100% backwards compatible and templates would not need to be changed.

Chris.


On 21 June 2012 19:20, Rouven Weßling <m...@rouvenwessling.de> wrote:
create_function isn't much better than eval, many hosts disable that one as well.

The check for only one parameter would be nice thou, that should eliminate the need for eval on most sites.

Rouven

--
You received this message because you are subscribed to the Google Groups "Joomla! CMS Development" group.
To post to this group, send an email to joomla-dev-cms@googlegroups.com.
To unsubscribe from this group, send email to joomla-dev-cms+unsubscribe@googlegroups.com.

For more options, visit this group at http://groups.google.com/group/joomla-dev-cms?hl=en-GB.

Reply all
Reply to author
Forward
0 new messages