Skip to content

Commit 29768cb

Browse files
dotani1111dotani1111
authored
Merge commit from fork
[4.3.1] MFAバイパス脆弱性の修正
2 parents cd8fa98 + 9df6353 commit 29768cb

4 files changed

Lines changed: 248 additions & 4 deletions

File tree

‎src/Eccube/Controller/Admin/Setting/System/TwoFactorAuthController.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,13 @@ public function set(Request $request)
109109
if (!$this->twoFactorAuthService->isEnabled() || $this->twoFactorAuthService->isAuth($Member)) {
110110
return $this->redirectToRoute('admin_homepage');
111111
}
112+
113+
// 既に2FAキーが設定されている場合は、認証画面にリダイレクト
114+
// 2FA未認証状態での再設定を防ぐ（MFAバイパス脆弱性対策）
115+
if ($Member->getTwoFactorAuthKey()) {
116+
return $this->redirectToRoute('admin_two_factor_auth');
117+
}
118+
112119
$res = $this->createResponse($request);
113120

114121
return $res;

‎src/Eccube/EventListener/TwoFactorAuthListener.php‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,12 @@ class TwoFactorAuthListener implements EventSubscriberInterface
2828
/**
2929
* @var array 2段階認証のチェックを除外するroute
3030
*/
31-
public const ROUTE_EXCLUDE = ['admin_two_factor_auth', 'admin_two_factor_auth_set'];
31+
public const ROUTE_EXCLUDE = ['admin_two_factor_auth'];
32+
33+
/**
34+
* @var array 2段階認証キー未設定時のみ除外するroute
35+
*/
36+
public const ROUTE_EXCLUDE_WHEN_NOT_CONFIGURED = ['admin_two_factor_auth_set'];
3237

3338
/**
3439
* @var EccubeConfig
@@ -89,9 +94,18 @@ public function onKernelController(ControllerArgumentsEvent $event)
8994
return;
9095
}
9196

97+
$Member = $this->requestContext->getCurrentUser();
98+
99+
// 2FAキー未設定時のみ除外するルートのチェック
100+
// 既に2FAキーが設定されている場合は除外しない（認証が必要）
101+
if (in_array($route, self::ROUTE_EXCLUDE_WHEN_NOT_CONFIGURED)) {
102+
if ($Member instanceof Member && !$Member->getTwoFactorAuthKey()) {
103+
return;
104+
}
105+
}
106+
92107
if (
93-
($Member = $this->requestContext->getCurrentUser())
94-
&& $Member instanceof Member
108+
$Member instanceof Member
95109
&& $Member->isTwoFactorAuthEnabled()
96110
&& !$this->twoFactorAuthService->isAuth($Member)
97111
) {

‎src/Eccube/Service/TwoFactorAuthService.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,10 @@ public function __construct(
102102
*/
103103
public function isAuth($Member)
104104
{
105-
if (($json = $this->request->cookies->get($this->cookieName))) {
105+
// テスト環境ではコンストラクタ時点でリクエストが存在しない場合があるため、
106+
// requestStackから現在のリクエストを再取得する
107+
$request = $this->request ?? $this->requestStack->getCurrentRequest();
108+
if ($request && $json = $request->cookies->get($this->cookieName)) {
106109
$configs = json_decode($json);
107110
$hasher = $this->passwordHasherFactory->getPasswordHasher($Member);
108111

‎tests/Eccube/Tests/Web/Admin/Setting/System/TwoFactorAuthControllerTest.php‎

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
<?php
2+
3+
/*
4+
* This file is part of EC-CUBE
5+
*
6+
* Copyright(c) EC-CUBE CO.,LTD. All Rights Reserved.
7+
*
8+
* http://www.ec-cube.co.jp/
9+
*
10+
* For the full copyright and license information, please view the LICENSE
11+
* file that was distributed with this source code.
12+
*/
13+
14+
namespace Eccube\Tests\Web\Admin\Setting\System;
15+
16+
use Eccube\Entity\Member;
17+
use Eccube\Repository\MemberRepository;
18+
use Eccube\Service\TwoFactorAuthService;
19+
use Eccube\Tests\Web\Admin\AbstractAdminWebTestCase;
20+
use RobThree\Auth\TwoFactorAuth;
21+
22+
class TwoFactorAuthControllerTest extends AbstractAdminWebTestCase
23+
{
24+
/**
25+
* @var MemberRepository
26+
*/
27+
protected $memberRepository;
28+
29+
/**
30+
* @var TwoFactorAuthService
31+
*/
32+
protected $twoFactorAuthService;
33+
34+
/**
35+
* @var TwoFactorAuth
36+
*/
37+
protected $tfa;
38+
39+
protected function setUp(): void
40+
{
41+
parent::setUp();
42+
$this->memberRepository = $this->entityManager->getRepository(Member::class);
43+
$this->twoFactorAuthService = static::getContainer()->get(TwoFactorAuthService::class);
44+
$this->tfa = new TwoFactorAuth();
45+
}
46+
47+
/**
48+
* 正常系1: 2FA有効済み・登録済みユーザーの認証成功
49+
* 管理画面 → ログイン → 2FA認証 → ログイン成功
50+
*/
51+
public function testAuthSuccessWithRegisteredUser()
52+
{
53+
if (!$this->twoFactorAuthService->isEnabled()) {
54+
$this->markTestSkipped('2FAが無効のためスキップ');
55+
}
56+
57+
// 2FA設定済みの新規メンバーを作成
58+
$authKey = $this->twoFactorAuthService->createSecret();
59+
$Member = $this->createMember();
60+
$Member->setTwoFactorAuthEnabled(true);
61+
$Member->setTwoFactorAuthKey($authKey);
62+
$this->entityManager->persist($Member);
63+
$this->entityManager->flush();
64+
65+
// 新しいMemberでログイン
66+
$this->loginTo($Member);
67+
68+
// 2FA認証画面にアクセス（CSRFトークン取得のため）
69+
$crawler = $this->client->request('GET', $this->generateUrl('admin_two_factor_auth'));
70+
$this->assertTrue($this->client->getResponse()->isSuccessful());
71+
72+
// CSRFトークンを取得
73+
$token = $crawler->filter('input[name="admin_two_factor_auth[_token]"]')->attr('value');
74+
75+
// 正しいTOTPコードを生成して送信
76+
$validCode = $this->tfa->getCode($authKey);
77+
$this->client->request('POST', $this->generateUrl('admin_two_factor_auth'), [
78+
'admin_two_factor_auth' => [
79+
'device_token' => $validCode,
80+
'_token' => $token,
81+
],
82+
]);
83+
84+
// ホームにリダイレクトされることを確認（ログイン成功）
85+
$this->assertTrue(
86+
$this->client->getResponse()->isRedirect($this->generateUrl('admin_homepage')),
87+
'2FA認証成功後、ホームにリダイレクトされるべき'
88+
);
89+
}
90+
91+
/**
92+
* 正常系2: 2FA有効済み・未登録ユーザーのセットアップ成功
93+
* 管理画面 → ログイン → 2FAセットアップ画面 → セットアップ完了
94+
*/
95+
public function testSetupSuccessWithUnregisteredUser()
96+
{
97+
if (!$this->twoFactorAuthService->isEnabled()) {
98+
$this->markTestSkipped('2FAが無効のためスキップ');
99+
}
100+
101+
// 2FA未設定の新規メンバーを作成
102+
$Member = $this->createMember();
103+
$Member->setTwoFactorAuthEnabled(true);
104+
$Member->setTwoFactorAuthKey(null); // 未登録
105+
$this->entityManager->persist($Member);
106+
$this->entityManager->flush();
107+
108+
// 新しいMemberでログイン
109+
$this->loginTo($Member);
110+
111+
// 2FAセットアップ画面にアクセス
112+
$crawler = $this->client->request('GET', $this->generateUrl('admin_two_factor_auth_set'));
113+
$this->assertTrue($this->client->getResponse()->isSuccessful());
114+
115+
// フォームから秘密鍵とCSRFトークンを取得
116+
$authKey = $crawler->filter('input[name="admin_two_factor_auth[auth_key]"]')->attr('value');
117+
$token = $crawler->filter('input[name="admin_two_factor_auth[_token]"]')->attr('value');
118+
119+
// 正しいTOTPコードを生成して送信
120+
$validCode = $this->tfa->getCode($authKey);
121+
$this->client->request('POST', $this->generateUrl('admin_two_factor_auth_set'), [
122+
'admin_two_factor_auth' => [
123+
'device_token' => $validCode,
124+
'auth_key' => $authKey,
125+
'_token' => $token,
126+
],
127+
]);
128+
129+
// ホームにリダイレクトされることを確認（セットアップ完了）
130+
$this->assertTrue(
131+
$this->client->getResponse()->isRedirect($this->generateUrl('admin_homepage')),
132+
'2FAセットアップ完了後、ホームにリダイレクトされるべき'
133+
);
134+
135+
// 2FAキーが保存されていることを確認（DBから再取得）
136+
$savedMember = $this->memberRepository->find($Member->getId());
137+
$this->assertNotNull($savedMember->getTwoFactorAuthKey(), '2FAキーが保存されているべき');
138+
}
139+
140+
/**
141+
* 失敗系: 2FA認証失敗
142+
* 間違ったコードで認証を試みた場合、エラーメッセージが表示される
143+
*/
144+
public function testAuthFailureWithInvalidCode()
145+
{
146+
if (!$this->twoFactorAuthService->isEnabled()) {
147+
$this->markTestSkipped('2FAが無効のためスキップ');
148+
}
149+
150+
// 2FA設定済みの新規メンバーを作成
151+
$authKey = $this->twoFactorAuthService->createSecret();
152+
$Member = $this->createMember();
153+
$Member->setTwoFactorAuthEnabled(true);
154+
$Member->setTwoFactorAuthKey($authKey);
155+
$this->entityManager->persist($Member);
156+
$this->entityManager->flush();
157+
158+
// 新しいMemberでログイン
159+
$this->loginTo($Member);
160+
161+
// 2FA認証画面にアクセス
162+
$crawler = $this->client->request('GET', $this->generateUrl('admin_two_factor_auth'));
163+
$this->assertTrue($this->client->getResponse()->isSuccessful());
164+
165+
// CSRFトークンを取得
166+
$token = $crawler->filter('input[name="admin_two_factor_auth[_token]"]')->attr('value');
167+
168+
// 間違ったコードを送信
169+
$crawler = $this->client->request('POST', $this->generateUrl('admin_two_factor_auth'), [
170+
'admin_two_factor_auth' => [
171+
'device_token' => '000000', // 無効なコード
172+
'_token' => $token,
173+
],
174+
]);
175+
176+
// リダイレクトされずに同じ画面に留まる（エラー表示）
177+
$this->assertFalse(
178+
$this->client->getResponse()->isRedirect(),
179+
'無効なコードの場合、リダイレクトされないべき'
180+
);
181+
182+
// エラーメッセージが表示されていることを確認（text-dangerクラスで表示）
183+
$errorElement = $crawler->filter('.text-danger');
184+
$this->assertGreaterThan(0, $errorElement->count(), 'エラーメッセージが表示されるべき');
185+
}
186+
187+
/**
188+
* 脆弱性テスト: MFAバイパス脆弱性
189+
* 2FAキーが設定済みのユーザーが2FA未認証状態で設定画面にアクセスした場合、
190+
* 認証画面にリダイレクトされることを確認
191+
*/
192+
public function testSetRedirectsToAuthWhenTwoFactorAuthKeyAlreadyConfigured()
193+
{
194+
// 2FAが無効な場合はスキップ
195+
if (!$this->twoFactorAuthService->isEnabled()) {
196+
$this->markTestSkipped('2FAが無効のためスキップ');
197+
}
198+
199+
// 2FA設定済みの新規メンバーを作成
200+
$Member = $this->createMember();
201+
$Member->setTwoFactorAuthEnabled(true);
202+
$Member->setTwoFactorAuthKey($this->twoFactorAuthService->createSecret());
203+
$this->entityManager->persist($Member);
204+
$this->entityManager->flush();
205+
206+
// 新しいMemberでログインし直す
207+
$this->loginTo($Member);
208+
209+
// 2FA未認証状態で設定画面にアクセス
210+
$this->client->request('GET', $this->generateUrl('admin_two_factor_auth_set'));
211+
212+
$response = $this->client->getResponse();
213+
214+
// 認証画面にリダイレクトされることを確認
215+
$this->assertTrue(
216+
$response->isRedirect($this->generateUrl('admin_two_factor_auth')),
217+
'2FAキー設定済みユーザーが未認証で設定画面にアクセスした場合、認証画面にリダイレクトされるべき。実際のレスポンス: Status='.$response->getStatusCode().', Location='.$response->headers->get('Location')
218+
);
219+
}
220+
}

0 commit comments

Comments
 (0)