Skip to content
This repository has been archived by the owner on Nov 26, 2017. It is now read-only.

Count modules security patch #1307

Closed
wants to merge 4 commits into from
Closed

Count modules security patch #1307

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
011253a
Merge remote-tracking branch 'joomla/master'
gamort Jun 19, 2012
e433121
Merge remote-tracking branch 'joomla/master'
gamort Jun 20, 2012
62e9cbf
Eliminates an eval call when not needed & makes sure operators for co…
gamort Jun 22, 2012
6803bae
Merge remote-tracking branch 'joomla/master' into countModulesSecurit…
gamort Jun 22, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions libraries/joomla/document/html/html.php
View file
Open in desktop
Expand Up @@ -484,6 +484,23 @@ public function countModules($condition)
{ {
$operators = '(\+|\-|\*|\/|==|\!=|\<\>|\<|\>|\<=|\>=|and|or|xor)'; $operators = '(\+|\-|\*|\/|==|\!=|\<\>|\<|\>|\<=|\>=|and|or|xor)';
$words = preg_split('# ' . $operators . ' #', $condition, null, PREG_SPLIT_DELIM_CAPTURE); $words = preg_split('# ' . $operators . ' #', $condition, null, PREG_SPLIT_DELIM_CAPTURE);

// $words must be odd, an even number of words is a mistake so skip processing
if (!(count($words) & 1)) {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd throw an InvalidArgumentException here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From deploying Joomla! CMS, I'd rather not have an exception thrown which breaks the website due to some invalid template...especially if it was in an admin template which then made it impossible to fix without directly updating database.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we can't do it in 2.5. But throwing an exception will make it much easier for devs to even notice their error so I think that is preferable for 3.0.

}

// don't allow undocumented/malicious operators
for ($i = 1, $n = count($words); $i < $n; $i += 2)
{
// even parts (operators)
$operator = strtolower($words[$i]);
if (!preg_match($operators, $words) )
{
return false;
}
}

for ($i = 0, $n = count($words); $i < $n; $i += 2) for ($i = 0, $n = count($words); $i < $n; $i += 2)
{ {
// Odd parts (modules) // Odd parts (modules)
Expand All @@ -493,6 +510,12 @@ public function countModules($condition)
: count(JModuleHelper::getModules($name)); : count(JModuleHelper::getModules($name));
} }



// one word doesn't need an eval call
if (count($words) == 1) {
return $words[0];
}

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


return eval($str); return eval($str);
Expand Down