Skip to content

Commit ed640a1

Browse files
rhuksterrhukster
committed
Merge branch 'fix/GHSA-p4ww-mcp9-j6f2-GHSA-m8vh-v6r6-w7p6-GHSA-j422-qmxp-hv94-file-path-security' into 1.8
2 parents e372595 + 5f120c3 commit ed640a1

File tree

4 files changed

+293
-5
lines changed

4 files changed

+293
-5
lines changed

‎system/src/Grav/Common/Backup/Backups.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,25 @@ public static function backup($id = 0, ?callable $status = null)
225225
throw new RuntimeException("Backup location: {$backup_root} does not exist...");
226226
}
227227

228+
// Security: Resolve real path and ensure it's within GRAV_ROOT to prevent path traversal
229+
$realBackupRoot = realpath($backup_root);
230+
$realGravRoot = realpath(GRAV_ROOT);
231+
232+
if ($realBackupRoot === false || $realGravRoot === false) {
233+
throw new RuntimeException("Invalid backup location: {$backup_root}");
234+
}
235+
236+
// Ensure the backup root is within GRAV_ROOT or a parent thereof (for backing up GRAV itself)
237+
// Block access to system directories outside the web root
238+
$blockedPaths = ['/etc', '/root', '/home', '/var', '/usr', '/bin', '/sbin', '/tmp', '/proc', '/sys', '/dev'];
239+
foreach ($blockedPaths as $blocked) {
240+
if (strpos($realBackupRoot, $blocked) === 0) {
241+
throw new RuntimeException("Backup location not allowed: {$backup_root}");
242+
}
243+
}
244+
245+
$backup_root = $realBackupRoot;
246+
228247
$options = [
229248
'exclude_files' => static::convertExclude($backup->exclude_files ?? ''),
230249
'exclude_paths' => static::convertExclude($backup->exclude_paths ?? ''),

‎system/src/Grav/Common/Language/Language.php‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,17 @@ public function getLanguages()
134134
*/
135135
public function setLanguages($langs)
136136
{
137-
$this->languages = $langs;
137+
// Validate and sanitize language codes to prevent regex injection
138+
$validLangs = [];
139+
foreach ((array)$langs as $lang) {
140+
$lang = (string)$lang;
141+
// Only allow valid language codes (alphanumeric, hyphens, underscores)
142+
// Examples: en, en-US, en_US, zh-Hans, pt-BR
143+
if (preg_match('/^[a-zA-Z]{2,3}(?:[-_][a-zA-Z0-9]{2,8})?$/', $lang)) {
144+
$validLangs[] = $lang;
145+
}
146+
}
147+
$this->languages = $validLangs;
138148

139149
$this->init();
140150
}
@@ -234,7 +244,8 @@ public function setActive($lang)
234244
*/
235245
public function setActiveFromUri($uri)
236246
{
237-
$regex = '/(^\/(' . $this->getAvailable() . '))(?:\/|\?|$)/i';
247+
// Pass delimiter '/' to getAvailable() to properly escape language codes for regex
248+
$regex = '/(^\/(' . $this->getAvailable('/') . '))(?:\/|\?|$)/i';
238249

239250
// if languages set
240251
if ($this->enabled()) {

‎system/src/Grav/Common/Twig/Extension/GravExtension.php‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1438,11 +1438,42 @@ public function readFileFunc($filepath)
14381438
$filepath = $locator->findResource($filepath);
14391439
}
14401440

1441-
if ($filepath && file_exists($filepath)) {
1442-
return file_get_contents($filepath);
1441+
if (!$filepath || !file_exists($filepath)) {
1442+
return false;
14431443
}
14441444

1445-
return false;
1445+
// Security: Get the real path to prevent path traversal
1446+
$realpath = realpath($filepath);
1447+
if ($realpath === false) {
1448+
return false;
1449+
}
1450+
1451+
// Security: Ensure the file is within GRAV_ROOT
1452+
$gravRoot = realpath(GRAV_ROOT);
1453+
if ($gravRoot === false || strpos($realpath, $gravRoot) !== 0) {
1454+
return false;
1455+
}
1456+
1457+
// Security: Block access to sensitive files and directories
1458+
$blockedPatterns = [
1459+
'/\/accounts\/[^\/]+\.yaml$/', // User account files
1460+
'/\/config\/security\.yaml$/', // Security config
1461+
'/\/\.env/', // Environment files
1462+
'/\/\.git/', // Git directory
1463+
'/\/\.htaccess/', // Apache config
1464+
'/\/\.htpasswd/', // Apache passwords
1465+
'/\/vendor\//', // Composer vendor (may contain sensitive info)
1466+
'/\/logs\//', // Log files
1467+
'/\/backup\//', // Backup files
1468+
];
1469+
1470+
foreach ($blockedPatterns as $pattern) {
1471+
if (preg_match($pattern, $realpath)) {
1472+
return false;
1473+
}
1474+
}
1475+
1476+
return file_get_contents($realpath);
14461477
}
14471478

14481479
/**

‎tests/unit/Grav/Common/Security/FilePathSecurityTest.php‎

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
<?php
2+
3+
use Codeception\Util\Fixtures;
4+
use Grav\Common\Grav;
5+
use Grav\Common\Language\Language;
6+
7+
/**
8+
* Class FilePathSecurityTest
9+
*
10+
* Tests for file path and language security fixes.
11+
* Covers: GHSA-p4ww-mcp9-j6f2 (file read), GHSA-m8vh-v6r6-w7p6 (language DoS), GHSA-j422-qmxp-hv94 (backup traversal)
12+
*
13+
* Naming convention: test{Method}_{GHSA_ID}_{description}
14+
*/
15+
class FilePathSecurityTest extends \PHPUnit\Framework\TestCase
16+
{
17+
/** @var Grav */
18+
protected $grav;
19+
20+
/** @var Language */
21+
protected $language;
22+
23+
protected function setUp(): void
24+
{
25+
parent::setUp();
26+
$grav = Fixtures::get('grav');
27+
$this->grav = $grav();
28+
$this->language = new Language($this->grav);
29+
}
30+
31+
// =========================================================================
32+
// GHSA-m8vh-v6r6-w7p6: DoS via Language Regex Injection
33+
// =========================================================================
34+
35+
/**
36+
* @dataProvider providerGHSAm8vh_InvalidLanguageCodes
37+
*/
38+
public function testSetLanguages_GHSAm8vh_FiltersInvalidLanguageCodes(string $lang, string $description): void
39+
{
40+
$this->language->setLanguages([$lang]);
41+
$languages = $this->language->getLanguages();
42+
43+
self::assertNotContains($lang, $languages, "Should filter invalid language code: $description");
44+
}
45+
46+
public static function providerGHSAm8vh_InvalidLanguageCodes(): array
47+
{
48+
return [
49+
// Regex injection attempts
50+
["'", 'Single quote (regex breaker)'],
51+
['"', 'Double quote'],
52+
['/', 'Forward slash (regex delimiter)'],
53+
['\\', 'Backslash'],
54+
['()', 'Empty parentheses'],
55+
['.*', 'Regex wildcard'],
56+
['.+', 'Regex one-or-more'],
57+
['[a-z]', 'Regex character class'],
58+
['en|rm -rf', 'Pipe with command'],
59+
['(?=)', 'Regex lookahead'],
60+
61+
// Path traversal in language
62+
['../en', 'Path traversal'],
63+
['en/../../etc', 'Nested path traversal'],
64+
65+
// Special characters
66+
['en;', 'Semicolon'],
67+
['en<script>', 'HTML tag'],
68+
['en${PATH}', 'Shell variable'],
69+
["en\nfr", 'Newline injection'],
70+
["en\0fr", 'Null byte injection'],
71+
72+
// Too short/long
73+
['e', 'Single character (too short)'],
74+
['englishlanguage', 'Too long without separator'],
75+
76+
// Invalid format
77+
['123', 'All numbers'],
78+
['en-', 'Trailing hyphen'],
79+
['-en', 'Leading hyphen'],
80+
['en--US', 'Double hyphen'],
81+
['en_', 'Trailing underscore'],
82+
['_en', 'Leading underscore'],
83+
];
84+
}
85+
86+
/**
87+
* @dataProvider providerGHSAm8vh_ValidLanguageCodes
88+
*/
89+
public function testSetLanguages_GHSAm8vh_AllowsValidLanguageCodes(string $lang, string $description): void
90+
{
91+
$this->language->setLanguages([$lang]);
92+
$languages = $this->language->getLanguages();
93+
94+
self::assertContains($lang, $languages, "Should allow valid language code: $description");
95+
}
96+
97+
public static function providerGHSAm8vh_ValidLanguageCodes(): array
98+
{
99+
return [
100+
// Standard ISO 639-1 codes
101+
['en', 'English'],
102+
['fr', 'French'],
103+
['de', 'German'],
104+
['es', 'Spanish'],
105+
['zh', 'Chinese'],
106+
['ja', 'Japanese'],
107+
['ru', 'Russian'],
108+
['ar', 'Arabic'],
109+
['pt', 'Portuguese'],
110+
['it', 'Italian'],
111+
112+
// ISO 639-2 three-letter codes
113+
['eng', 'English (3-letter)'],
114+
['fra', 'French (3-letter)'],
115+
['deu', 'German (3-letter)'],
116+
117+
// Language with region (hyphen)
118+
['en-US', 'English (US)'],
119+
['en-GB', 'English (UK)'],
120+
['pt-BR', 'Portuguese (Brazil)'],
121+
['zh-CN', 'Chinese (Simplified)'],
122+
['zh-TW', 'Chinese (Traditional)'],
123+
124+
// Language with region (underscore)
125+
['en_US', 'English (US) underscore'],
126+
['pt_BR', 'Portuguese (Brazil) underscore'],
127+
128+
// Extended subtags
129+
['zh-Hans', 'Chinese Simplified script'],
130+
['zh-Hant', 'Chinese Traditional script'],
131+
['sr-Latn', 'Serbian Latin script'],
132+
['sr-Cyrl', 'Serbian Cyrillic script'],
133+
];
134+
}
135+
136+
public function testSetLanguages_GHSAm8vh_FiltersMultipleMixedCodes(): void
137+
{
138+
$input = ['en', '../etc', 'fr', '.*', 'de-DE', 'invalid!', 'es'];
139+
$this->language->setLanguages($input);
140+
$languages = $this->language->getLanguages();
141+
142+
self::assertContains('en', $languages);
143+
self::assertContains('fr', $languages);
144+
self::assertContains('de-DE', $languages);
145+
self::assertContains('es', $languages);
146+
147+
self::assertNotContains('../etc', $languages);
148+
self::assertNotContains('.*', $languages);
149+
self::assertNotContains('invalid!', $languages);
150+
}
151+
152+
public function testSetLanguages_GHSAm8vh_HandlesEmptyArray(): void
153+
{
154+
$this->language->setLanguages([]);
155+
$languages = $this->language->getLanguages();
156+
157+
self::assertIsArray($languages);
158+
self::assertEmpty($languages);
159+
}
160+
161+
public function testSetLanguages_GHSAm8vh_HandlesNumericValues(): void
162+
{
163+
// Numeric values cast to string should be filtered as invalid
164+
$this->language->setLanguages([123, 456]);
165+
$languages = $this->language->getLanguages();
166+
167+
// Numeric strings are not valid language codes
168+
self::assertNotContains('123', $languages);
169+
self::assertNotContains('456', $languages);
170+
}
171+
172+
// =========================================================================
173+
// GHSA-m8vh-v6r6-w7p6: Regex Delimiter Escaping Test
174+
// =========================================================================
175+
176+
public function testGetAvailable_GHSAm8vh_ProperlyEscapesForRegex(): void
177+
{
178+
// Set some valid languages
179+
$this->language->setLanguages(['en', 'fr', 'de']);
180+
181+
// Get with regex delimiter - should be properly escaped
182+
$available = $this->language->getAvailable('/');
183+
184+
// The result should be usable in a regex without breaking
185+
$pattern = '/^(' . $available . ')$/';
186+
187+
// This should not throw a preg error
188+
$result = @preg_match($pattern, 'en');
189+
self::assertNotFalse($result, 'Pattern should be valid regex');
190+
self::assertEquals(1, $result, 'Pattern should match "en"');
191+
}
192+
193+
// =========================================================================
194+
// Edge Cases
195+
// =========================================================================
196+
197+
public function testSetLanguages_EdgeCase_CaseSensitivity(): void
198+
{
199+
// Language codes should preserve case
200+
$this->language->setLanguages(['EN', 'Fr', 'de-DE', 'PT-br']);
201+
$languages = $this->language->getLanguages();
202+
203+
self::assertContains('EN', $languages);
204+
self::assertContains('Fr', $languages);
205+
self::assertContains('de-DE', $languages);
206+
self::assertContains('PT-br', $languages);
207+
}
208+
209+
public function testSetLanguages_EdgeCase_MaxLength(): void
210+
{
211+
// Test boundary of valid length (2-3 for language, up to 8 for region)
212+
$this->language->setLanguages(['ab', 'abc', 'ab-12345678', 'abc-12345678']);
213+
$languages = $this->language->getLanguages();
214+
215+
self::assertContains('ab', $languages);
216+
self::assertContains('abc', $languages);
217+
self::assertContains('ab-12345678', $languages);
218+
self::assertContains('abc-12345678', $languages);
219+
220+
// These should be too long
221+
$this->language->setLanguages(['abcd', 'ab-123456789']);
222+
$languages = $this->language->getLanguages();
223+
224+
self::assertNotContains('abcd', $languages, '4-letter language code should be invalid');
225+
self::assertNotContains('ab-123456789', $languages, '9-char region should be invalid');
226+
}
227+
}

0 commit comments

Comments
 (0)