From 2e13ae746fd16664e16412a7b904378369db0cf7 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 10 Apr 2018 13:46:08 +1200 Subject: [PATCH 1/2] [ss-2018-006] Prevent code execution in template value resolution --- src/Dev/FixtureBlueprint.php | 2 +- src/Forms/GridField/GridFieldDataColumns.php | 2 +- src/ORM/Hierarchy/MarkedSet.php | 2 +- src/View/SSViewer_DataPresenter.php | 2 +- tests/php/View/SSViewerTest.php | 10 ++++++++++ 5 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/Dev/FixtureBlueprint.php b/src/Dev/FixtureBlueprint.php index 9d219a36f..47e6a1805 100644 --- a/src/Dev/FixtureBlueprint.php +++ b/src/Dev/FixtureBlueprint.php @@ -121,7 +121,7 @@ class FixtureBlueprint continue; } - if (is_callable($fieldVal)) { + if (!is_string($fieldVal) && is_callable($fieldVal)) { $obj->$fieldName = $fieldVal($obj, $data, $fixtures); } else { $obj->$fieldName = $fieldVal; diff --git a/src/Forms/GridField/GridFieldDataColumns.php b/src/Forms/GridField/GridFieldDataColumns.php index 623515323..c8dd223da 100644 --- a/src/Forms/GridField/GridFieldDataColumns.php +++ b/src/Forms/GridField/GridFieldDataColumns.php @@ -281,7 +281,7 @@ class GridFieldDataColumns implements GridField_ColumnProvider } $spec = $this->fieldFormatting[$fieldName]; - if (is_callable($spec)) { + if (!is_string($spec) && is_callable($spec)) { return $spec($value, $item); } else { $format = str_replace('$value', "__VAL__", $spec); diff --git a/src/ORM/Hierarchy/MarkedSet.php b/src/ORM/Hierarchy/MarkedSet.php index 96fd23602..52b287a63 100644 --- a/src/ORM/Hierarchy/MarkedSet.php +++ b/src/ORM/Hierarchy/MarkedSet.php @@ -333,7 +333,7 @@ class MarkedSet $parentNode->setField('markingClasses', $this->markingClasses($data['node'])); // Evaluate custom context - if (is_callable($context)) { + if (!is_array($context) && is_callable($context)) { $context = call_user_func($context, $data['node']); } if ($context) { diff --git a/src/View/SSViewer_DataPresenter.php b/src/View/SSViewer_DataPresenter.php index cf609d117..f8d120cd8 100644 --- a/src/View/SSViewer_DataPresenter.php +++ b/src/View/SSViewer_DataPresenter.php @@ -326,7 +326,7 @@ class SSViewer_DataPresenter extends SSViewer_Scope $override = $overrides[$property]; // Late-evaluate this value - if (is_callable($override)) { + if (!is_string($override) && is_callable($override)) { $override = $override(); // Late override may yet return null diff --git a/tests/php/View/SSViewerTest.php b/tests/php/View/SSViewerTest.php index 098a2272b..8f79afe14 100644 --- a/tests/php/View/SSViewerTest.php +++ b/tests/php/View/SSViewerTest.php @@ -109,6 +109,16 @@ class SSViewerTest extends SapphireTest $this->assertEquals('Test partial template: var value', trim(preg_replace("//U", '', $result))); } + /** + * Ensure global methods aren't executed + */ + public function testTemplateExecution() + { + $data = new ArrayData([ 'Var' => 'phpinfo' ]); + $result = $data->renderWith("SSViewerTestPartialTemplate"); + $this->assertEquals('Test partial template: phpinfo', trim(preg_replace("//U", '', $result))); + } + public function testIncludeScopeInheritance() { $data = $this->getScopeInheritanceTestData(); From cd716fb61b58ac9e059310c223491e366a47ee23 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Thu, 10 May 2018 10:32:00 +1200 Subject: [PATCH 2/2] Switch check for is_string --- src/ORM/Hierarchy/MarkedSet.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ORM/Hierarchy/MarkedSet.php b/src/ORM/Hierarchy/MarkedSet.php index 52b287a63..a2adda78d 100644 --- a/src/ORM/Hierarchy/MarkedSet.php +++ b/src/ORM/Hierarchy/MarkedSet.php @@ -333,7 +333,7 @@ class MarkedSet $parentNode->setField('markingClasses', $this->markingClasses($data['node'])); // Evaluate custom context - if (!is_array($context) && is_callable($context)) { + if (!is_string($context) && is_callable($context)) { $context = call_user_func($context, $data['node']); } if ($context) {