From e03ccbd7a9d0d46b05adb0362a974889ec8efd52 Mon Sep 17 00:00:00 2001
From: Kijin Sung <kijin@kijinsung.com>
Date: Tue, 18 Sep 2018 02:45:48 +0900
Subject: [PATCH 1/2] Also filter XML and HTML file uploads

- Detect possible XML (including SVG) by actual content, not extension
- Check large files in overlapping chunks to reduce memory usage
- Check XML files for external entities
- Check HTML files for SSI and PHP code
---
 classes/security/UploadFileFilter.class.php | 107 +++++++++++++++++---
 1 file changed, 94 insertions(+), 13 deletions(-)

diff --git a/classes/security/UploadFileFilter.class.php b/classes/security/UploadFileFilter.class.php
index 1b3b73a34..38843005f 100644
--- a/classes/security/UploadFileFilter.class.php
+++ b/classes/security/UploadFileFilter.class.php
@@ -1,5 +1,4 @@
 <?php
-/* Copyright (C) NAVER <http://www.navercorp.com> */
 
 class UploadFileFilter
 {
@@ -19,7 +18,7 @@ class UploadFileFilter
 		}
 		
 		// Return error if the file size is zero.
-		if (!filesize($file))
+		if (($filesize = filesize($file)) == 0)
 		{
 			return false;
 		}
@@ -27,38 +26,120 @@ class UploadFileFilter
 		// Get the extension.
 		$ext = $filename ? strtolower(substr(strrchr($filename, '.'), 1)) : '';
 		
+		// Check the first 4KB of the file for possible XML content.
+		$fp = fopen($file, 'rb');
+		$first4kb = fread($fp, 4096);
+		$is_xml = preg_match('/<(?:\?xml|!DOCTYPE|html|head|body|meta|script|svg)\b/i', $first4kb);
+		
 		// Check SVG files.
-		if ($ext === 'svg' && !self::_checkSVG($file))
+		if (($ext === 'svg' || $is_xml) && !self::_checkSVG($fp, 0, $filesize))
 		{
+			fclose($fp);
+			return false;
+		}
+		
+		// Check XML files.
+		if (($ext === 'xml' || $is_xml) && !self::_checkXML($fp, 0, $filesize))
+		{
+			fclose($fp);
+			return false;
+		}
+		
+		// Check HTML files.
+		if (($ext === 'html' || $ext === 'shtml' || $ext === 'xhtml' || $ext === 'phtml' || $is_xml) && !self::_checkHTML($fp, 0, $filesize))
+		{
+			fclose($fp);
 			return false;
 		}
 		
 		// Return true if everything is OK.
+		fclose($fp);
 		return true;
 	}
 	
 	/**
 	 * Check SVG file for XSS or SSRF vulnerabilities (#1088, #1089)
 	 *
-	 * @param string $file
+	 * @param resource $fp
+	 * @param int $from
+	 * @param int $to
 	 * @return bool
 	 */
-	protected static function _checkSVG($file)
+	protected static function _checkSVG($fp, $from, $to)
 	{
-		$content = file_get_contents($file);
-		
-		if (preg_match('/xlink:href\s*=\s*"(?!data:)/i', $content))
-		{
-			return false;
-		}
-		
-		if (preg_match('/<script/i', $content))
+		if (self::_matchStream('/<script|xlink:href\s*=\s*"(?!data:)/i', $fp, $from, $to))
 		{
 			return false;
 		}
 		
 		return true;
 	}
+	
+	/**
+	 * Check XML file for external entity inclusion.
+	 *
+	 * @param resource $fp
+	 * @param int $from
+	 * @param int $to
+	 * @return bool
+	 */
+	protected static function _checkXML($fp, $from, $to)
+	{
+		if (self::_matchStream('/<!ENTITY/i', $fp, $from, $to))
+		{
+			return false;
+		}
+		
+		return true;
+	}
+	
+	/**
+	 * Check HTML file for PHP code, server-side includes, and other nastiness.
+	 *
+	 * @param resource $fp
+	 * @param int $from
+	 * @param int $to
+	 * @return bool
+	 */
+	protected static function _checkHTML($fp, $from, $to)
+	{
+		if (self::_matchStream('/<\?(?!xml\b)|<!--#(?:include|exec|echo|config|fsize|flastmod|printenv)\b/i', $fp, $from, $to))
+		{
+			return false;
+		}
+		
+		return true;
+	}
+	
+	/**
+	 * Match a stream against a regular expression.
+	 * 
+	 * This method is useful when dealing with large files,
+	 * because we don't need to load the entire file into memory.
+	 * We allow a generous overlap in case the matching string
+	 * occurs across a block boundary.
+	 * 
+	 * @param string $regexp
+	 * @param resource $fp
+	 * @param int $from
+	 * @param int $to
+	 * @param int $block_size (optional)
+	 * @param int $overlap_size (optional)
+	 * @return bool
+	 */
+	protected static function _matchStream($regexp, $fp, $from, $to, $block_size = 16384, $overlap_size = 1024)
+	{
+		fseek($fp, $position = $from);
+		while (strlen($content = fread($fp, $block_size + $overlap_size)) > 0)
+		{
+			if (preg_match($regexp, $content))
+			{
+				return true;
+			}
+			fseek($fp, min($to, $position += $block_size));
+		}
+		return false;
+	}
 }
 
 /* End of file : UploadFileFilter.class.php */

From 19331e6746cceb9a23bc48e7ff48963674155847 Mon Sep 17 00:00:00 2001
From: Kijin Sung <kijin@kijinsung.com>
Date: Tue, 18 Sep 2018 13:27:55 +0900
Subject: [PATCH 2/2] Additional XSS checks #1088

---
 classes/context/Context.class.php           | 17 +++++++----------
 classes/security/UploadFileFilter.class.php |  6 +++++-
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/classes/context/Context.class.php b/classes/context/Context.class.php
index 86c33c211..ba96dda4a 100644
--- a/classes/context/Context.class.php
+++ b/classes/context/Context.class.php
@@ -1407,20 +1407,17 @@ class Context
 			}
 			elseif($_val = trim($_val))
 			{
-				if(in_array($key, array('page', 'cpage')) || ends_with('srl', $key, false))
+				if(in_array($key, array('page', 'cpage')) || ends_with('srl', $key, false) && preg_match('/[^0-9,]/', $_val))
 				{
-					if(preg_match('/[^0-9,]/', $_val))
-					{
-						$_val = (int)$_val;
-					}
+					$_val = (int)$_val;
 				}
-				elseif(in_array($key, array('mid', 'search_keyword', 'xe_validator_id')))
+				elseif(in_array($key, array('mid', 'vid', 'search_target', 'search_keyword', 'xe_validator_id')) || count($_GET))
 				{
 					$_val = escape($_val, false);
-				}
-				elseif($key === 'vid')
-				{
-					$_val = urlencode($_val);
+					if(ends_with('url', $key, false))
+					{
+						$_val = strtr($_val, array('&amp;' => '&'));
+					}
 				}
 			}
 			$result[escape($_key)] = $_val;
diff --git a/classes/security/UploadFileFilter.class.php b/classes/security/UploadFileFilter.class.php
index 38843005f..85aae19ba 100644
--- a/classes/security/UploadFileFilter.class.php
+++ b/classes/security/UploadFileFilter.class.php
@@ -67,7 +67,11 @@ class UploadFileFilter
 	 */
 	protected static function _checkSVG($fp, $from, $to)
 	{
-		if (self::_matchStream('/<script|xlink:href\s*=\s*"(?!data:)/i', $fp, $from, $to))
+		if (self::_matchStream('/<script|<handler\b|xlink:href\s*=\s*"(?!data:)/i', $fp, $from, $to))
+		{
+			return false;
+		}
+		if (self::_matchStream('/\b(?:ev:(?:event|listener|observer)|on[a-z]+)\s*=/i', $fp, $from, $to))
 		{
 			return false;
 		}