Skip to content

Commit f0f8e8c

Browse files
committed
fix: validate LESS feature usage in theme config settings
CVE-2023-27577 restricted @import and data-uri() in the custom_less setting, but the same restriction was never applied to other settings registered as LESS config variables (e.g. theme_primary_color). Those values are interpolated verbatim into the LESS source, allowing an admin-level attacker to read local files or trigger SSRF via the LESS @import directive. Apply the same check to every dirty setting whose key is registered via flarum.less.config (built-in theme colors and Extend\Settings::registerLessConfigVar()).
1 parent 85fd06c commit f0f8e8c

2 files changed

Lines changed: 78 additions & 6 deletions

File tree

framework/core/src/Forum/ValidateCustomLess.php

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,26 @@ public function whenSettingsSaving(Saving $event)
6262
return;
6363
}
6464

65-
// Restrict what features can be used in custom LESS
66-
if (isset($event->settings['custom_less']) && preg_match('/@import|data-uri\s*\(/i', $event->settings['custom_less'])) {
67-
$translator = $this->container->make(TranslatorInterface::class);
65+
// Restrict what features can be used in custom LESS. This applies to
66+
// the `custom_less` setting as well as any setting registered as a
67+
// LESS config variable (e.g. `theme_primary_color`), since those
68+
// values are interpolated directly into the LESS source.
69+
$lessFeatureKeys = array_merge(
70+
isset($event->settings['custom_less']) ? ['custom_less'] : [],
71+
array_intersect(
72+
array_keys($event->settings),
73+
array_column($this->customLessSettings, 'key')
74+
)
75+
);
76+
77+
foreach ($lessFeatureKeys as $key) {
78+
if (is_string($event->settings[$key]) && preg_match('/@import|data-uri\s*\(/i', $event->settings[$key])) {
79+
$translator = $this->container->make(TranslatorInterface::class);
6880

69-
throw new ValidationException([
70-
'custom_less' => $translator->trans('core.admin.appearance.custom_styles_cannot_use_less_features')
71-
]);
81+
throw new ValidationException([
82+
$key => $translator->trans('core.admin.appearance.custom_styles_cannot_use_less_features')
83+
]);
84+
}
7285
}
7386

7487
// We haven't saved the settings yet, but we want to trial a full

framework/core/tests/integration/api/settings/SetTest.php

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,4 +82,63 @@ public function max_setting_length_validated()
8282

8383
$this->assertEquals(422, $response->getStatusCode());
8484
}
85+
86+
/**
87+
* @test
88+
*/
89+
public function theme_primary_color_rejects_less_import()
90+
{
91+
$response = $this->send(
92+
$this->request('POST', '/api/settings', [
93+
'authenticatedAs' => 1,
94+
'json' => [
95+
'theme_primary_color' => "#4D698E;@import (inline) '/etc/passwd';",
96+
],
97+
])
98+
);
99+
100+
$this->assertEquals(422, $response->getStatusCode());
101+
$this->assertNotEquals(
102+
"#4D698E;@import (inline) '/etc/passwd';",
103+
$this->app->getContainer()->make('flarum.settings')->get('theme_primary_color')
104+
);
105+
}
106+
107+
/**
108+
* @test
109+
*/
110+
public function theme_secondary_color_rejects_less_import()
111+
{
112+
$response = $this->send(
113+
$this->request('POST', '/api/settings', [
114+
'authenticatedAs' => 1,
115+
'json' => [
116+
'theme_secondary_color' => "#4D698E;@import (inline) '/etc/passwd';",
117+
],
118+
])
119+
);
120+
121+
$this->assertEquals(422, $response->getStatusCode());
122+
$this->assertNotEquals(
123+
"#4D698E;@import (inline) '/etc/passwd';",
124+
$this->app->getContainer()->make('flarum.settings')->get('theme_secondary_color')
125+
);
126+
}
127+
128+
/**
129+
* @test
130+
*/
131+
public function theme_primary_color_rejects_data_uri()
132+
{
133+
$response = $this->send(
134+
$this->request('POST', '/api/settings', [
135+
'authenticatedAs' => 1,
136+
'json' => [
137+
'theme_primary_color' => "#4D698E;background:data-uri('/etc/passwd');",
138+
],
139+
])
140+
);
141+
142+
$this->assertEquals(422, $response->getStatusCode());
143+
}
85144
}

0 commit comments

Comments
 (0)