From: Tim Düsterhus <duesterhus@woltlab.com>
Date: Fri, 19 Aug 2016 14:41:39 +0000 (+0200)
Subject: Fix RCE vulnerability in image proxy
X-Git-Tag: 3.0.0_Beta_1~657
X-Git-Url: https://git.stricted.de/?a=commitdiff_plain;h=29d0faf2881403896462ac1617e3c5632f4b154f;p=GitHub%2FWoltLab%2FWCF.git

Fix RCE vulnerability in image proxy
---

diff --git a/wcfsetup/install/files/lib/action/ImageProxyAction.class.php b/wcfsetup/install/files/lib/action/ImageProxyAction.class.php
index 6e56668dac..959986174a 100644
--- a/wcfsetup/install/files/lib/action/ImageProxyAction.class.php
+++ b/wcfsetup/install/files/lib/action/ImageProxyAction.class.php
@@ -46,18 +46,19 @@ class ImageProxyAction extends AbstractAction {
 			if ($url === null) throw new IllegalLinkException();
 			
 			$fileName = sha1($this->key);
+			$dir = WCF_DIR.'images/proxy/'.substr($fileName, 0, 2);
 			
-			// prepare path
-			$fileExtension = pathinfo($url, PATHINFO_EXTENSION);
-			$path = 'images/proxy/'.substr($fileName, 0, 2).'/'.$fileName.($fileExtension ? '.'.$fileExtension : '');
-			$fileLocation = WCF_DIR.$path;
-			$dir = dirname($fileLocation);
-			if (!@file_exists($dir)) {
+			// ensure that the directory exists
+			if (!file_exists($dir)) {
 				FileUtil::makePath($dir);
 			}
 			
-			// download image
-			if (!file_exists($fileLocation)) {
+			// check whether we already downloaded the image
+			$files = glob($dir.'/'.$fileName.'.{png,gif,jpg}', GLOB_BRACE | GLOB_NOSORT);
+			if ($files === false) throw new IllegalLinkException();
+			
+			if (empty($files)) {
+				// download image
 				try {
 					$request = new HTTPRequest($url);
 					$request->execute();
@@ -67,18 +68,37 @@ class ImageProxyAction extends AbstractAction {
 				}
 				$image = $request->getReply()['body'];
 				
-				// check if image is linked
-				// TODO: handle SVGs
+				// check file type
 				$imageData = getimagesizefromstring($image);
-				if (!$imageData) {
-					throw new IllegalLinkException();
+				if (!$imageData) throw new IllegalLinkException();
+				
+				switch ($imageData[2]) {
+					case IMAGETYPE_PNG:
+						$extension = 'png';
+					break;
+					case IMAGETYPE_GIF:
+						$extension = 'gif';
+					break;
+					case IMAGETYPE_JPEG:
+						$extension = 'jpg';
+					break;
+					default:
+						throw new IllegalLinkException();
 				}
 				
+				$fileLocation = $dir.'/'.$fileName.'.'.$extension;
+				
 				file_put_contents($fileLocation, $image);
 				
 				// update mtime for correct expiration calculation
 				@touch($fileLocation);
 			}
+			else {
+				$fileLocation = $files[0];
+			}
+			
+			$path = FileUtil::getRelativePath(WCF_DIR, dirname($fileLocation)).basename($fileLocation);
+			
 			$this->executed();
 			
 			HeaderUtil::redirect(WCF::getPath().$path, true, false);
diff --git a/wcfsetup/install/files/lib/system/bbcode/ImageBBCode.class.php b/wcfsetup/install/files/lib/system/bbcode/ImageBBCode.class.php
index 176a522beb..e95ce344f2 100644
--- a/wcfsetup/install/files/lib/system/bbcode/ImageBBCode.class.php
+++ b/wcfsetup/install/files/lib/system/bbcode/ImageBBCode.class.php
@@ -59,8 +59,7 @@ class ImageBBCode extends AbstractBBCode {
 	}
 	
 	/**
-	 * Returns the link to the cached image (or the link to fetch the image
-	 * using the image proxy).
+	 * Returns the link to fetch the image using the image proxy.
 	 * 
 	 * @param	string		$link
 	 * @return	string
@@ -69,17 +68,6 @@ class ImageBBCode extends AbstractBBCode {
 	protected function getProxyLink($link) {
 		try {
 			$key = CryptoUtil::createSignedString($link);
-			// does not need to be secure, just sufficiently "random"
-			$fileName = sha1($key);
-			
-			$fileExtension = pathinfo($this->url, PATHINFO_EXTENSION);
-			
-			$path = 'images/proxy/'.substr($fileName, 0, 2).'/'.$fileName.($fileExtension ? '.'.$fileExtension : '');
-			
-			$fileLocation = WCF_DIR.$path;
-			if (file_exists($fileLocation)) {
-				return WCF::getPath().$path;
-			}
 			
 			return LinkHandler::getInstance()->getLink('ImageProxy', [
 				'key' => $key
diff --git a/wcfsetup/install/files/lib/system/html/output/node/HtmlOutputNodeImg.class.php b/wcfsetup/install/files/lib/system/html/output/node/HtmlOutputNodeImg.class.php
index 90f1e5b183..a78fa774de 100644
--- a/wcfsetup/install/files/lib/system/html/output/node/HtmlOutputNodeImg.class.php
+++ b/wcfsetup/install/files/lib/system/html/output/node/HtmlOutputNodeImg.class.php
@@ -39,8 +39,7 @@ class HtmlOutputNodeImg extends AbstractHtmlOutputNode {
 	}
 	
 	/**
-	 * Returns the link to the cached image (or the link to fetch the image
-	 * using the image proxy).
+	 * Returns the link to fetch the image using the image proxy.
 	 *
 	 * @param	string		$link
 	 * @return	string
@@ -49,17 +48,6 @@ class HtmlOutputNodeImg extends AbstractHtmlOutputNode {
 	protected function getProxyLink($link) {
 		try {
 			$key = CryptoUtil::createSignedString($link);
-			// does not need to be secure, just sufficiently "random"
-			$fileName = sha1($key);
-			
-			$fileExtension = pathinfo($link, PATHINFO_EXTENSION);
-			
-			$path = 'images/proxy/'.substr($fileName, 0, 2).'/'.$fileName.($fileExtension ? '.'.$fileExtension : '');
-			
-			$fileLocation = WCF_DIR.$path;
-			if (file_exists($fileLocation)) {
-				return WCF::getPath().$path;
-			}
 			
 			return LinkHandler::getInstance()->getLink('ImageProxy', [
 				'key' => $key