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 6e56668dacddc29c17b367116bd77811fbcd9507..959986174a81b3be8b3fad58acb96af0fe6244c0 100644 (file)
--- 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 176a522beb55339682de1c3be32ac76a7ad14de0..e95ce344f27804515fa18560662a142cecfb8573 100644 (file)
--- 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 90f1e5b183b4991e9bd5b90b21dace483a63cf45..a78fa774de5896c72c4c6713f05f684f103704b0 100644 (file)
--- 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