From f8b0ca6a78ff37f2d78d0faf6e144f9547e99c98 Mon Sep 17 00:00:00 2001
From: Kijin Sung <kijin@kijinsung.com>
Date: Sat, 13 Oct 2018 16:42:19 +0900
Subject: [PATCH] Improve granularity of Context security check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

정규식에 따라 모두 막을지, 관리자만 허용할지 구분할 수 있도록 하고
이를 참조하여 <script> 태그는 관리자만 허용, PHP 태그는 모두 막도록 변경.
---
 classes/context/Context.class.php      | 26 +++++++++++++++-----------
 classes/module/ModuleHandler.class.php | 18 +++++++++++++++---
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/classes/context/Context.class.php b/classes/context/Context.class.php
index bc12764b7..c37f4c23c 100644
--- a/classes/context/Context.class.php
+++ b/classes/context/Context.class.php
@@ -117,10 +117,10 @@ class Context
 	public $is_site_locked = FALSE;
 
 	/**
-	 * Check init
-	 * @var bool FALSE if init fail
+	 * Result of initial security check
+	 * @var string|bool
 	 */
-	public $isSuccessInit = TRUE;
+	public $security_check = 'OK';
 
 	/**
 	 * Singleton instance
@@ -173,8 +173,9 @@ class Context
 	 * @var array
 	 */
 	private static $_check_patterns = array(
-		'@<(?:\?|%)@',
-		'@<script\s*?language\s*?=@i',
+		'@<(?:\?|%)@' => 'DENY ALL',
+		'@<script\s*?language\s*?=@i' => 'DENY ALL',
+		'@</?script@i' => 'ALLOW ADMIN ONLY',
 	);
 
 	/**
@@ -1070,7 +1071,7 @@ class Context
 	{
 		if (!self::_recursiveCheckVar($_SERVER['HTTP_HOST']) || preg_match("/[\,\"\'\{\}\[\]\(\);$]/", $_SERVER['HTTP_HOST']))
 		{
-			self::$_instance->isSuccessInit = FALSE;
+			self::$_instance->security_check = 'DENY ALL';
 		}
 	}
 
@@ -1267,7 +1268,7 @@ class Context
 				}
 				if(!UploadFileFilter::check($tmp_name, $val['name']))
 				{
-					self::$_instance->isSuccessInit = false;
+					self::$_instance->security_check = 'DENY ALL';
 					unset($_FILES[$key]);
 					continue;
 				}
@@ -1287,7 +1288,7 @@ class Context
 					}
 					if(!UploadFileFilter::check($val['tmp_name'][$i], $val['name'][$i]))
 					{
-						self::$_instance->isSuccessInit = false;
+						self::$_instance->security_check = 'DENY ALL';
 						$files = array();
 						unset($_FILES[$key]);
 						break;
@@ -1318,12 +1319,15 @@ class Context
 	{
 		if(is_string($val))
 		{
-			foreach(self::$_check_patterns as $pattern)
+			foreach(self::$_check_patterns as $pattern => $status)
 			{
 				if(preg_match($pattern, $val))
 				{
-					self::$_instance->isSuccessInit = false;
-					return false;
+					self::$_instance->security_check = $status;
+					if($status === 'DENY ALL')
+					{
+						return false;
+					}
 				}
 			}
 		}
diff --git a/classes/module/ModuleHandler.class.php b/classes/module/ModuleHandler.class.php
index 9bfaccde7..3523375bf 100644
--- a/classes/module/ModuleHandler.class.php
+++ b/classes/module/ModuleHandler.class.php
@@ -42,11 +42,23 @@ class ModuleHandler extends Handler
 			return;
 		}
 
+		// Check security check status
 		$oContext = Context::getInstance();
-		if($oContext->isSuccessInit === false)
+		switch($oContext->security_check)
 		{
-			$this->error = 'msg_security_violation';
-			return;
+			case 'OK':
+				break;
+			case 'ALLOW ADMIN ONLY':
+				if(!Context::get('logged_info')->isAdmin())
+				{
+					$this->error = 'msg_security_violation';
+					return;
+				}
+				break;
+			case 'DENY ALL':
+			default:
+				$this->error = 'msg_security_violation';
+				return;
 		}
 
 		// Set variables from request arguments