Skip to content

Commit 934da43

Browse files
nanasessnanasess
authored
Merge pull request #1341 from EC-CUBE/fix/security-rce-dynamic-include
Fix RCE: アップデート/バッチ機能の動的includeによるコード実行脆弱性を修正
2 parents a87c5e2 + 4fe8745 commit 934da43

File tree

3 files changed

+327
-6
lines changed

3 files changed

+327
-6
lines changed

‎data/class/batch/SC_Batch_Update.php‎

Lines changed: 74 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public function execute($target = '.')
5858
$bkupPathFile = $bkupPath.'files/';
5959
$this->lfMkdirRecursive($bkupPathFile.'dummy');
6060

61+
$distinfo = [];
6162
$arrLog = [
6263
'err' => [],
6364
'ok' => [],
@@ -84,12 +85,9 @@ public function execute($target = '.')
8485
// 拡張子を取得
8586
$suffix = pathinfo($path, PATHINFO_EXTENSION);
8687

87-
// distinfo の変数定義
88-
$distinfo ??= '';
89-
9088
// distinfo.php を読み込む
9189
if ($fileName == 'distinfo.php') {
92-
include_once $path;
90+
$distinfo = $this->parseDistInfo($path);
9391
}
9492

9593
// 除外ファイルをスキップ
@@ -252,16 +250,86 @@ public function lfMkdirRecursive($path)
252250
public function makeDistInfo($bkupDistInfoArray)
253251
{
254252
$src = "<?php\n"
255-
.'$distifo = array('."\n";
253+
.'$distinfo = array('."\n";
256254

257255
foreach ($bkupDistInfoArray as $key => $value) {
258-
$src .= "'{$key}' => '{$value}',\n";
256+
$src .= "'".addcslashes($key, "'")."' => '".addcslashes($value, "'")."',\n";
259257
}
260258
$src .= ");\n?>";
261259

262260
return $src;
263261
}
264262

263+
/**
264+
* distinfo.php を安全にパースして $distinfo 配列を返す.
265+
*
266+
* include を使わず、ファイル内容を正規表現でパースすることで
267+
* 任意コード実行を防止する.
268+
*
269+
* @param string $path distinfo.php のパス
270+
*
271+
* @return array SHA1ハッシュ => ファイルパス の連想配列
272+
*/
273+
public function parseDistInfo($path)
274+
{
275+
$content = file_get_contents($path);
276+
if ($content === false) {
277+
$this->printLog('distinfoファイルの読み込みに失敗しました: '.$path);
278+
279+
return [];
280+
}
281+
282+
// PHP定数の解決マップ
283+
$constants = [];
284+
foreach (['MODULE_REALDIR', 'HTML_REALDIR', 'DATA_REALDIR'] as $name) {
285+
if (defined($name)) {
286+
$constants[$name] = constant($name);
287+
}
288+
}
289+
290+
$distinfo = [];
291+
// 'sha1hash' => MODULE_REALDIR . 'filepath', または 'sha1hash' => 'filepath', の形式をパースする
292+
if (preg_match_all("/'([a-f0-9]{40})'\s*=>\s*(.+?)\s*[,)]/", $content, $matches)) {
293+
$count = count($matches[0]);
294+
for ($i = 0; $i < $count; $i++) {
295+
$value = trim($matches[2][$i]);
296+
// CONSTANT . 'path' の形式を解決する (e.g., MODULE_REALDIR . 'mdl_foo/path.php')
297+
if (preg_match('/^([A-Z_]+)\s*\.\s*([\'"])(.+?)\2$/', $value, $valMatch)) {
298+
$constName = $valMatch[1];
299+
$relativePath = $valMatch[3];
300+
if (isset($constants[$constName])) {
301+
// パストラバーサルを防止
302+
if (str_contains($relativePath, '..') || (isset($relativePath[0]) && $relativePath[0] === '/')) {
303+
$this->printLog('不正な相対パスが検出されました: '.$relativePath);
304+
} else {
305+
$distinfo[$matches[1][$i]] = $constants[$constName].$relativePath;
306+
}
307+
} else {
308+
$this->printLog('未定義の定数が使用されています: '.$constName);
309+
}
310+
} elseif (preg_match('/^([\'"])(.+?)\1$/', $value, $valMatch)) {
311+
// 'filepath' の形式 (バックアップファイル等)
312+
$literalPath = $valMatch[2];
313+
// 既知のEC-CUBEベースディレクトリ配下か検証
314+
$isValidBase = false;
315+
foreach ($constants as $base) {
316+
if (str_starts_with($literalPath, $base)) {
317+
$isValidBase = true;
318+
break;
319+
}
320+
}
321+
if ($isValidBase) {
322+
$distinfo[$matches[1][$i]] = $literalPath;
323+
} else {
324+
$this->printLog('不正なパスが検出されました: '.$literalPath);
325+
}
326+
}
327+
}
328+
}
329+
330+
return $distinfo;
331+
}
332+
265333
/**
266334
* @param string $msg
267335
*/

‎data/class/pages/upgrade/LC_Page_Upgrade_Download.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,12 @@ public function registerUpdateLog($arrLog, $objRet)
403403
*/
404404
public function fileExecute($productCode)
405405
{
406+
if (!preg_match('/^[a-zA-Z0-9_-]+$/', $productCode)) {
407+
GC_Utils_Ex::gfPrintLog('Invalid product code: '.str_replace(["\r", "\n"], '', $productCode));
408+
409+
return;
410+
}
411+
406412
$file = DATA_REALDIR.'downloads/update/'.$productCode.'_update.php';
407413
if (file_exists($file)) {
408414
@include_once $file;

‎tests/class/batch/SC_Batch_Update_parseDistInfoTest.php‎

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
<?php
2+
3+
/**
4+
* @backupGlobals disabled
5+
*/
6+
class SC_Batch_Update_parseDistInfoTest extends PHPUnit_Framework_TestCase
7+
{
8+
/** @var SC_Batch_Update */
9+
private $batch;
10+
11+
/** @var string */
12+
private $tmpDir;
13+
14+
protected function setUp(): void
15+
{
16+
if (!defined('MODULE_REALDIR') || !defined('HTML_REALDIR') || !defined('DATA_REALDIR')) {
17+
$this->markTestSkipped('EC-CUBE constants are not defined.');
18+
}
19+
$this->batch = new SC_Batch_Update();
20+
$this->tmpDir = sys_get_temp_dir().'/sc_batch_update_test_'.uniqid();
21+
mkdir($this->tmpDir, 0777, true);
22+
// セキュリティテスト用のセンチネルファイルが残っていれば事前削除
23+
if (file_exists('/tmp/pwned.txt')) {
24+
unlink('/tmp/pwned.txt');
25+
}
26+
}
27+
28+
protected function tearDown(): void
29+
{
30+
// テンポラリファイルを削除 (setUp がスキップした場合は tmpDir が null のため guard が必要)
31+
if ($this->tmpDir !== null && is_dir($this->tmpDir)) {
32+
array_map('unlink', glob($this->tmpDir.'/*') ?: []);
33+
rmdir($this->tmpDir);
34+
}
35+
if (file_exists('/tmp/pwned.txt')) {
36+
unlink('/tmp/pwned.txt');
37+
}
38+
}
39+
40+
/**
41+
* オーナーズストアが生成する distinfo.php のフォーマットをパースできること
42+
*/
43+
public function testオーナーズストア形式の定数連結をパースできる()
44+
{
45+
$content = <<<'PHP'
46+
<?php
47+
$distinfo = array(
48+
'da39a3ee5e6b4b0d3255bfef95601890afd80709' => MODULE_REALDIR . 'mdl_example/example.php',
49+
'a94a8fe5ccb19ba61c4c0873d391e987982fbbd3' => MODULE_REALDIR . 'mdl_example/config.php',
50+
);
51+
?>
52+
PHP;
53+
$path = $this->tmpDir.'/distinfo.php';
54+
file_put_contents($path, $content);
55+
56+
$result = $this->batch->parseDistInfo($path);
57+
58+
$this->assertCount(2, $result);
59+
$this->assertSame(
60+
MODULE_REALDIR.'mdl_example/example.php',
61+
$result['da39a3ee5e6b4b0d3255bfef95601890afd80709']
62+
);
63+
$this->assertSame(
64+
MODULE_REALDIR.'mdl_example/config.php',
65+
$result['a94a8fe5ccb19ba61c4c0873d391e987982fbbd3']
66+
);
67+
}
68+
69+
/**
70+
* makeDistInfo() が生成するバックアップ用フォーマットをパースできること
71+
*/
72+
public function testバックアップ形式の文字列リテラルをパースできる()
73+
{
74+
// makeDistInfo() が生成する形式を再現（リテラルパスはEC-CUBEベースディレクトリ配下）
75+
$filePath1 = DATA_REALDIR.'class/example.php';
76+
$filePath2 = HTML_REALDIR.'index.php';
77+
$content = "<?php\n\$distinfo = array(\n"
78+
."'da39a3ee5e6b4b0d3255bfef95601890afd80709' => '{$filePath1}',\n"
79+
."'a94a8fe5ccb19ba61c4c0873d391e987982fbbd3' => '{$filePath2}',\n"
80+
.");\n?>";
81+
$path = $this->tmpDir.'/distinfo.php';
82+
file_put_contents($path, $content);
83+
84+
$result = $this->batch->parseDistInfo($path);
85+
86+
$this->assertCount(2, $result);
87+
$this->assertSame(
88+
$filePath1,
89+
$result['da39a3ee5e6b4b0d3255bfef95601890afd80709']
90+
);
91+
$this->assertSame(
92+
$filePath2,
93+
$result['a94a8fe5ccb19ba61c4c0873d391e987982fbbd3']
94+
);
95+
}
96+
97+
/**
98+
* HTML_REALDIR 定数の連結をパースできること
99+
*/
100+
public function testHTMLREALDIR定数の連結をパースできる()
101+
{
102+
$content = <<<'PHP'
103+
<?php
104+
$distinfo = array(
105+
'da39a3ee5e6b4b0d3255bfef95601890afd80709' => HTML_REALDIR . 'js/example.js',
106+
);
107+
?>
108+
PHP;
109+
$path = $this->tmpDir.'/distinfo.php';
110+
file_put_contents($path, $content);
111+
112+
$result = $this->batch->parseDistInfo($path);
113+
114+
$this->assertCount(1, $result);
115+
$this->assertSame(
116+
HTML_REALDIR.'js/example.js',
117+
$result['da39a3ee5e6b4b0d3255bfef95601890afd80709']
118+
);
119+
}
120+
121+
/**
122+
* 存在しないファイルの場合は空配列を返すこと
123+
*/
124+
public function test存在しないファイルは空配列を返す()
125+
{
126+
$result = $this->batch->parseDistInfo($this->tmpDir.'/nonexistent.php');
127+
128+
$this->assertSame([], $result);
129+
}
130+
131+
/**
132+
* 空のファイルの場合は空配列を返すこと
133+
*/
134+
public function test空ファイルは空配列を返す()
135+
{
136+
$path = $this->tmpDir.'/distinfo.php';
137+
file_put_contents($path, '<?php ?>');
138+
139+
$result = $this->batch->parseDistInfo($path);
140+
141+
$this->assertSame([], $result);
142+
}
143+
144+
/**
145+
* 悪意あるPHPコードを含む distinfo.php が実行されないこと
146+
*/
147+
public function test悪意あるコードが実行されない()
148+
{
149+
$content = <<<'PHP'
150+
<?php
151+
system('echo PWNED > /tmp/pwned.txt');
152+
$distinfo = array(
153+
'da39a3ee5e6b4b0d3255bfef95601890afd80709' => MODULE_REALDIR . 'mdl_example/example.php',
154+
);
155+
?>
156+
PHP;
157+
$path = $this->tmpDir.'/distinfo.php';
158+
file_put_contents($path, $content);
159+
160+
$result = $this->batch->parseDistInfo($path);
161+
162+
// パースは正常に動作する
163+
$this->assertCount(1, $result);
164+
// 悪意あるコードは実行されていない
165+
$this->assertFileDoesNotExist('/tmp/pwned.txt');
166+
}
167+
168+
/**
169+
* 未定義の定数が使用されている場合はスキップされること
170+
*/
171+
public function test未定義の定数はスキップされる()
172+
{
173+
$content = <<<'PHP'
174+
<?php
175+
$distinfo = array(
176+
'da39a3ee5e6b4b0d3255bfef95601890afd80709' => UNKNOWN_CONST . 'path/to/file.php',
177+
);
178+
?>
179+
PHP;
180+
$path = $this->tmpDir.'/distinfo.php';
181+
file_put_contents($path, $content);
182+
183+
$result = $this->batch->parseDistInfo($path);
184+
185+
$this->assertSame([], $result);
186+
}
187+
188+
/**
189+
* パストラバーサルを含む相対パスがスキップされること
190+
*/
191+
public function testパストラバーサルを含むパスはスキップされる()
192+
{
193+
$content = <<<'PHP'
194+
<?php
195+
$distinfo = array(
196+
'da39a3ee5e6b4b0d3255bfef95601890afd80709' => MODULE_REALDIR . '../../html/upload/shell.php',
197+
);
198+
?>
199+
PHP;
200+
$path = $this->tmpDir.'/distinfo.php';
201+
file_put_contents($path, $content);
202+
203+
$result = $this->batch->parseDistInfo($path);
204+
205+
$this->assertSame([], $result);
206+
}
207+
208+
/**
209+
* 絶対パスで始まる相対パスがスキップされること
210+
*/
211+
public function test絶対パスで始まる相対パスはスキップされる()
212+
{
213+
$content = <<<'PHP'
214+
<?php
215+
$distinfo = array(
216+
'da39a3ee5e6b4b0d3255bfef95601890afd80709' => MODULE_REALDIR . '/etc/passwd',
217+
);
218+
?>
219+
PHP;
220+
$path = $this->tmpDir.'/distinfo.php';
221+
file_put_contents($path, $content);
222+
223+
$result = $this->batch->parseDistInfo($path);
224+
225+
$this->assertSame([], $result);
226+
}
227+
228+
/**
229+
* リテラルパスがEC-CUBEベースディレクトリ外の場合スキップされること
230+
*/
231+
public function testベースディレクトリ外のリテラルパスはスキップされる()
232+
{
233+
$content = <<<'PHP'
234+
<?php
235+
$distinfo = array(
236+
'da39a3ee5e6b4b0d3255bfef95601890afd80709' => '/etc/passwd',
237+
);
238+
?>
239+
PHP;
240+
$path = $this->tmpDir.'/distinfo.php';
241+
file_put_contents($path, $content);
242+
243+
$result = $this->batch->parseDistInfo($path);
244+
245+
$this->assertSame([], $result);
246+
}
247+
}

0 commit comments

Comments
 (0)