Skip to content

Commit fb88e6f

Browse files
bennothommobennothommo
authored
Merge commit from fork
* Harden theme objects, prevent certain properties from being passed through to ThemeData object * Improve and properly scope Twig security policy - Deny access to "include" and "extends" tags/functions, as per Markup docs - Block methods that write, delete or modify records and attributes in Database/Eloquent and Halcyon models - Block access to theme datasource - Prevent extensions from being created or directly interacted with (models and properties provided to extended objects should still be OK) * Minor * Make theme magic method calls case insensitive * Allow include and extends tags/functions * Add additional blocked methods * Add test cases
1 parent 5f53526 commit fb88e6f

File tree

4 files changed

+411
-38
lines changed

4 files changed

+411
-38
lines changed

‎modules/cms/classes/Theme.php‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,25 @@
1-
<?php namespace Cms\Classes;
1+
<?php
2+
3+
namespace Cms\Classes;
24

3-
use App;
4-
use ApplicationException;
5-
use Cache;
65
use Cms\Models\ThemeData;
7-
use Config;
86
use DirectoryIterator;
9-
use Event;
107
use Exception;
11-
use File;
12-
use Lang;
8+
use Illuminate\Support\Facades\App;
9+
use Illuminate\Support\Facades\Cache;
10+
use Illuminate\Support\Facades\Lang;
1311
use System\Models\Parameter;
14-
use SystemException;
15-
use Url;
12+
use Winter\Storm\Exception\ApplicationException;
13+
use Winter\Storm\Exception\SystemException;
1614
use Winter\Storm\Halcyon\Datasource\DatasourceInterface;
1715
use Winter\Storm\Halcyon\Datasource\DbDatasource;
1816
use Winter\Storm\Halcyon\Datasource\FileDatasource;
19-
use Yaml;
17+
use Winter\Storm\Support\Facades\Config;
18+
use Winter\Storm\Support\Facades\Event;
19+
use Winter\Storm\Support\Facades\File;
20+
use Winter\Storm\Support\Facades\Url;
21+
use Winter\Storm\Support\Facades\Yaml;
22+
use Winter\Storm\Support\Str;
2023

2124
/**
2225
* This class represents the CMS theme.
@@ -682,6 +685,11 @@ public function getDatasource(): DatasourceInterface
682685
*/
683686
public function __get($name)
684687
{
688+
if (in_array(strtolower($name), ['id', 'path', 'dirname', 'config', 'formconfig', 'previewimageurl'])) {
689+
$method = 'get'. ucfirst($name);
690+
return $this->$method();
691+
}
692+
685693
if ($this->hasCustomData()) {
686694
return $this->getCustomData()->{$name};
687695
}
@@ -694,6 +702,10 @@ public function __get($name)
694702
*/
695703
public function __isset($key)
696704
{
705+
if (in_array(strtolower($key), ['id', 'path', 'dirname', 'config', 'formconfig', 'previewimageurl'])) {
706+
return true;
707+
}
708+
697709
if ($this->hasCustomData()) {
698710
$theme = $this->getCustomData();
699711
return $theme->offsetExists($key);

‎modules/cms/models/ThemeData.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
<?php namespace Cms\Models;
1+
<?php
2+
3+
namespace Cms\Models;
24

3-
use Lang;
4-
use Model;
55
use Cms\Classes\Theme as CmsTheme;
6-
use System\Classes\CombineAssets;
76
use Exception;
7+
use Illuminate\Support\Facades\Lang;
8+
use System\Classes\CombineAssets;
89
use System\Models\File;
10+
use Winter\Storm\Database\Model;
911

1012
/**
1113
* Customization data used by a theme

‎modules/system/tests/twig/SecurityPolicyTest.php‎

Lines changed: 269 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,269 @@
1+
<?php
2+
3+
namespace System\Tests\Twig;
4+
5+
use Cms\Classes\Controller;
6+
use Cms\Classes\Page;
7+
use Cms\Classes\Theme;
8+
use System\Tests\Bootstrap\TestCase;
9+
use Twig\Environment;
10+
use Winter\Storm\Filesystem\Filesystem;
11+
use Winter\Storm\Halcyon\Datasource\FileDatasource;
12+
13+
class SecurityPolicyTest extends TestCase
14+
{
15+
protected Environment $twig;
16+
17+
public function testCannotGetTwigInstanceFromCmsController()
18+
{
19+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
20+
21+
$this->renderTwigInCmsController('
22+
{% set twig = this.controller.getTwig() %}
23+
{{ this.controller.getTwig() }}
24+
');
25+
}
26+
27+
public function testCannotGetTwigLoaderFromCmsController()
28+
{
29+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
30+
31+
$this->renderTwigInCmsController('
32+
{% set loader = this.controller.getLoader() %}
33+
{{ loader.load(\'/\') }}
34+
');
35+
}
36+
37+
public function testCannotRunAPageObjectFromWithinTwig()
38+
{
39+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
40+
41+
$this->renderTwigInCmsController('
42+
{{ this.controller.runPage() }}
43+
');
44+
}
45+
46+
public function testCannotExtendAPageWithADynamicMethod()
47+
{
48+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
49+
50+
$this->renderTwigInCmsController('
51+
{% set page = this.page.addDynamicMethod("test") %}
52+
');
53+
}
54+
55+
public function testCannotExtendAPageWithADynamicProperty()
56+
{
57+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
58+
59+
$this->renderTwigInCmsController('
60+
{% set page = this.page.addDynamicProperty("test", "value") %}
61+
');
62+
}
63+
64+
public function testCannotWriteToAModel()
65+
{
66+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
67+
68+
$this->renderTwigInCmsController('
69+
{% set modelTest = model.setAttribute("test", "value") %}
70+
', [
71+
'model' => new \Winter\Storm\Database\Model(),
72+
]);
73+
}
74+
75+
public function testCanReadFromAModel()
76+
{
77+
$model = new \Winter\Storm\Database\Model();
78+
$model->test = 'value';
79+
80+
$result = trim($this->renderTwigInCmsController('
81+
{% set modelTest = model.getAttribute("test") %}
82+
{{- modelTest -}}
83+
', [
84+
'model' => $model,
85+
]));
86+
$this->assertEquals('value', $result);
87+
}
88+
89+
public function testCannotFillAModel()
90+
{
91+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
92+
93+
try {
94+
$model = new \Winter\Storm\Database\Model();
95+
$model->addFillable('test');
96+
$model->test = 'value';
97+
98+
$this->renderTwigInCmsController('
99+
{% set modelTest = model.fill({ test: \'value2\' }) %}
100+
', [
101+
'model' => new \Winter\Storm\Database\Model(),
102+
]);
103+
} catch (\Twig\Sandbox\SecurityNotAllowedMethodError $e) {
104+
// Ensure value hasn't changed
105+
$this->assertEquals('value', $model->test);
106+
throw $e;
107+
}
108+
}
109+
110+
public function testCannotSaveAModel()
111+
{
112+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
113+
114+
$this->renderTwigInCmsController('
115+
{% set modelTest = model.save() %}
116+
', [
117+
'model' => new \Winter\Storm\Database\Model(),
118+
]);
119+
}
120+
121+
public function testCannotPushAModel()
122+
{
123+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
124+
125+
$this->renderTwigInCmsController('
126+
{% set modelTest = model.push() %}
127+
', [
128+
'model' => new \Winter\Storm\Database\Model(),
129+
]);
130+
}
131+
132+
public function testCannotUpdateAModel()
133+
{
134+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
135+
136+
$model = new \Winter\Storm\Database\Model();
137+
$model->addFillable('test');
138+
$model->test = 'value';
139+
140+
$this->renderTwigInCmsController('
141+
{% set modelTest = model.update({ test: \'value2\' }) %}
142+
', [
143+
'model' => $model,
144+
]);
145+
}
146+
147+
public function testCannotDeleteAModel()
148+
{
149+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
150+
151+
$this->renderTwigInCmsController('
152+
{% set modelTest = model.delete() %}
153+
', [
154+
'model' => new \Winter\Storm\Database\Model(),
155+
]);
156+
}
157+
158+
public function testCannotForceDeleteAModel()
159+
{
160+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
161+
162+
$this->renderTwigInCmsController('
163+
{% set modelTest = model.forceDelete() %}
164+
', [
165+
'model' => new \Winter\Storm\Database\Model(),
166+
]);
167+
}
168+
169+
public function testCannotExtendAModelWithABehaviour()
170+
{
171+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
172+
173+
$this->renderTwigInCmsController('
174+
{% set model = model.extendClassWith("Winter\Storm\Database\Behaviors\Encryptable") %}
175+
', [
176+
'model' => new \Winter\Storm\Database\Model(),
177+
]);
178+
}
179+
180+
public function testExtendingModelBeforePassingIntoTwigShouldStillWork()
181+
{
182+
$model = new \Winter\Storm\Database\Model();
183+
$model->addDynamicMethod('foo', function () {
184+
return 'foo';
185+
});
186+
187+
$result = trim($this->renderTwigInCmsController('
188+
{{- model.foo() -}}
189+
', [
190+
'model' => $model,
191+
]));
192+
$this->assertEquals('foo', $result);
193+
}
194+
195+
public function testCannotGetDatasourceFromTheme()
196+
{
197+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
198+
199+
$this->renderTwigInCmsController('
200+
{% set datasource = this.theme.getDatasource() %}
201+
');
202+
}
203+
204+
// Even if someone decides to be clever and make the datasource available, you shouldn't be able to insert/delete/update
205+
public function testCannotDeleteInDatasource()
206+
{
207+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
208+
209+
$this->renderTwigInCmsController('
210+
{% set datasource = datasource.delete() %}
211+
', [
212+
'datasource' => new FileDatasource(
213+
base_path('modules/system/tests/fixtures/themes/test'),
214+
new Filesystem()
215+
),
216+
]);
217+
}
218+
219+
public function testCannotInsertInDatasource()
220+
{
221+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
222+
223+
$this->renderTwigInCmsController('
224+
{% set datasource = datasource.insert() %}
225+
', [
226+
'datasource' => new FileDatasource(
227+
base_path('modules/system/tests/fixtures/themes/test'),
228+
new Filesystem()
229+
),
230+
]);
231+
}
232+
233+
public function testCannotUpdateInDatasource()
234+
{
235+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
236+
237+
$this->renderTwigInCmsController('
238+
{% set datasource = datasource.update() %}
239+
', [
240+
'datasource' => new FileDatasource(
241+
base_path('modules/system/tests/fixtures/themes/test'),
242+
new Filesystem()
243+
),
244+
]);
245+
}
246+
247+
public function testCannotChangeThemeDirectory()
248+
{
249+
$this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
250+
251+
$this->renderTwigInCmsController('
252+
{% set theme = this.theme.setDirName("test") %}
253+
');
254+
}
255+
256+
protected function renderTwigInCmsController(string $source, array $vars = [])
257+
{
258+
$controller = new Controller();
259+
$twig = $controller->getTwig();
260+
$template = $twig->createTemplate($source, 'test.case');
261+
return $twig->render($template, [
262+
'this' => [
263+
'controller' => $controller,
264+
'page' => new Page(),
265+
'theme' => new Theme()
266+
],
267+
] + $vars);
268+
}
269+
}

0 commit comments

Comments
 (0)