From: Lukas Reschke <lukas@owncloud.com>
Date: Thu, 11 Sep 2014 17:21:56 +0000 (+0200)
Subject: Do only follow HTTP and HTTPS redirects
X-Git-Tag: v8.0.0alpha1~624^2
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=6eeb905871fc7a671f99fd22c2592358a6abc02d;p=nextcloud-server.git

Do only follow HTTP and HTTPS redirects

We do not want to follow redirects to other protocols since they might allow an adversary to bypass network restrictions. (i.e. a redirect to ftp:// might be used to access files of a FTP server which might be in a secure zone and not be reachable from the net but from the ownCloud server)

Get final redirect manually using get_headers()

Migrate to HTTPHelper class and add unit tests
---

diff --git a/apps/files/ajax/newfile.php b/apps/files/ajax/newfile.php
index 46629e1b602..392fc5bd1c8 100644
--- a/apps/files/ajax/newfile.php
+++ b/apps/files/ajax/newfile.php
@@ -46,6 +46,7 @@ function progress($notification_code, $severity, $message, $message_code, $bytes
 	}
 }
 
+
 $l10n = \OC::$server->getL10N('files');
 
 $result = array(
@@ -93,7 +94,8 @@ if (\OC\Files\Filesystem::file_exists($target)) {
 }
 
 if($source) {
-	if(substr($source, 0, 8)!='https://' and substr($source, 0, 7)!='http://') {
+	$httpHelper = \OC::$server->getHTTPHelper();
+	if(!$httpHelper->isHTTPURL($source)) {
 		OCP\JSON::error(array('data' => array('message' => $l10n->t('Not a valid source'))));
 		exit();
 	}
@@ -104,7 +106,10 @@ if($source) {
 		exit();
 	}
 
-	$ctx = stream_context_create(null, array('notification' =>'progress'));
+	$source = $httpHelper->getFinalLocationOfURL($source);
+
+	$ctx = stream_context_create(\OC::$server->getHTTPHelper()->getDefaultContextArray(), array('notification' =>'progress'));
+
 	$sourceStream=@fopen($source, 'rb', false, $ctx);
 	$result = 0;
 	if (is_resource($sourceStream)) {
diff --git a/lib/private/files/storage/dav.php b/lib/private/files/storage/dav.php
index bd7166c9823..a0ef79a7b32 100644
--- a/lib/private/files/storage/dav.php
+++ b/lib/private/files/storage/dav.php
@@ -177,6 +177,8 @@ class DAV extends \OC\Files\Storage\Common {
 				curl_setopt($curl, CURLOPT_URL, $this->createBaseUri() . $this->encodePath($path));
 				curl_setopt($curl, CURLOPT_FILE, $fp);
 				curl_setopt($curl, CURLOPT_FOLLOWLOCATION, true);
+				curl_setopt($curl, CURLOPT_PROTOCOLS,  CURLPROTO_HTTP | CURLPROTO_HTTPS);
+				curl_setopt($curl, CURLOPT_REDIR_PROTOCOLS,  CURLPROTO_HTTP | CURLPROTO_HTTPS);
 				if ($this->secure === true) {
 					curl_setopt($curl, CURLOPT_SSL_VERIFYPEER, true);
 					curl_setopt($curl, CURLOPT_SSL_VERIFYHOST, 2);
@@ -282,6 +284,8 @@ class DAV extends \OC\Files\Storage\Common {
 		curl_setopt($curl, CURLOPT_INFILESIZE, filesize($path));
 		curl_setopt($curl, CURLOPT_PUT, true);
 		curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);
+		curl_setopt($curl, CURLOPT_PROTOCOLS,  CURLPROTO_HTTP | CURLPROTO_HTTPS);
+		curl_setopt($curl, CURLOPT_REDIR_PROTOCOLS,  CURLPROTO_HTTP | CURLPROTO_HTTPS);
 		if ($this->secure === true) {
 			curl_setopt($curl, CURLOPT_SSL_VERIFYPEER, true);
 			curl_setopt($curl, CURLOPT_SSL_VERIFYHOST, 2);
diff --git a/lib/private/httphelper.php b/lib/private/httphelper.php
new file mode 100644
index 00000000000..8b7aebb3d4d
--- /dev/null
+++ b/lib/private/httphelper.php
@@ -0,0 +1,177 @@
+<?php
+/**
+ * Copyright (c) 2014 Lukas Reschke <lukas@owncloud.com>
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace OC;
+
+class HTTPHelper {
+	const USER_AGENT = 'ownCloud Server Crawler';
+
+	/** @var \OC\AllConfig */
+	private $config;
+
+	/**
+	 * @param \OC\AllConfig $config
+	 */
+	public function __construct(AllConfig $config) {
+		$this->config = $config;
+	}
+
+	/**
+	 * Returns the default context array
+	 * @return array
+	 */
+	public function getDefaultContextArray() {
+		return array(
+			'http' => array(
+				'header' => 'User-Agent: ' . self::USER_AGENT . "\r\n",
+				'timeout' => 10,
+				'follow_location' => false, // Do not follow the location since we can't limit the protocol
+			),
+			'ssl' => array(
+				'disable_compression' => true
+			)
+		);
+	}
+
+	/**
+	 * Get URL content
+	 * @param string $url Url to get content
+	 * @throws \Exception If the URL does not start with http:// or https://
+	 * @return string of the response or false on error
+	 * This function get the content of a page via curl, if curl is enabled.
+	 * If not, file_get_contents is used.
+	 */
+	public function getUrlContent($url) {
+		if (!$this->isHTTPURL($url)) {
+			throw new \Exception('$url must start with https:// or http://', 1);
+		}
+
+		$proxy = $this->config->getSystemValue('proxy', null);
+		$proxyUserPwd = $this->config->getSystemValue('proxyuserpwd', null);
+		if (function_exists('curl_init')) {
+			$curl = curl_init();
+			$max_redirects = 10;
+
+			curl_setopt($curl, CURLOPT_HEADER, 0);
+			curl_setopt($curl, CURLOPT_RETURNTRANSFER, 1);
+			curl_setopt($curl, CURLOPT_CONNECTTIMEOUT, 10);
+			curl_setopt($curl, CURLOPT_URL, $url);
+			curl_setopt($curl, CURLOPT_PROTOCOLS,  CURLPROTO_HTTP | CURLPROTO_HTTPS);
+			curl_setopt($curl, CURLOPT_REDIR_PROTOCOLS,  CURLPROTO_HTTP | CURLPROTO_HTTPS);
+
+			curl_setopt($curl, CURLOPT_USERAGENT, self::USER_AGENT);
+			if ($proxy !== null) {
+				curl_setopt($curl, CURLOPT_PROXY, $proxy);
+			}
+			if ($proxyUserPwd !== null) {
+				curl_setopt($curl, CURLOPT_PROXYUSERPWD, $proxyUserPwd);
+			}
+
+			if (ini_get('open_basedir') === '' && (ini_get('safe_mode') === false) || strtolower(ini_get('safe_mode')) === 'off') {
+				curl_setopt($curl, CURLOPT_FOLLOWLOCATION, true);
+				curl_setopt($curl, CURLOPT_MAXREDIRS, $max_redirects);
+				$data = curl_exec($curl);
+			} else {
+				curl_setopt($curl, CURLOPT_FOLLOWLOCATION, false);
+				$mr = $max_redirects;
+				if ($mr > 0) {
+					$newURL = curl_getinfo($curl, CURLINFO_EFFECTIVE_URL);
+					$rcurl = curl_copy_handle($curl);
+					curl_setopt($rcurl, CURLOPT_HEADER, true);
+					curl_setopt($rcurl, CURLOPT_NOBODY, true);
+					curl_setopt($rcurl, CURLOPT_FORBID_REUSE, false);
+					curl_setopt($rcurl, CURLOPT_RETURNTRANSFER, true);
+					curl_setopt($rcurl, CURLOPT_USERAGENT, self::USER_AGENT);
+					do {
+						curl_setopt($rcurl, CURLOPT_URL, $newURL);
+						$header = curl_exec($rcurl);
+						if (curl_errno($rcurl)) {
+							$code = 0;
+						} else {
+							$code = curl_getinfo($rcurl, CURLINFO_HTTP_CODE);
+							if ($code == 301 || $code == 302) {
+								preg_match('/Location:(.*?)\n/', $header, $matches);
+								$newURL = trim(array_pop($matches));
+							} else {
+								$code = 0;
+							}
+						}
+					} while ($code && --$mr);
+					curl_close($rcurl);
+					if ($mr > 0) {
+						curl_setopt($curl, CURLOPT_URL, $newURL);
+					}
+				}
+
+				if ($mr == 0 && $max_redirects > 0) {
+					$data = false;
+				} else {
+					$data = curl_exec($curl);
+				}
+			}
+			curl_close($curl);
+		} else {
+			$url = $this->getFinalLocationOfURL($url);
+			$contextArray = $this->getDefaultContextArray();
+
+			if ($proxy !== null) {
+				$contextArray['http']['proxy'] = $proxy;
+			}
+
+			$ctx = stream_context_create(
+				$contextArray
+			);
+			$data = @file_get_contents($url, 0, $ctx);
+
+		}
+		return $data;
+	}
+
+	/**
+	 * Returns the response headers of a HTTP URL without following redirects
+	 * @param string $location Needs to be a HTTPS or HTTP URL
+	 * @return array
+	 */
+	public function getHeaders($location) {
+		stream_context_set_default($this->getDefaultContextArray());
+		return get_headers($location, 1);
+	}
+
+	/**
+	 * Checks whether the supplied URL begins with HTTPS:// or HTTP:// (case insensitive)
+	 * @param string $url
+	 * @return bool
+	 */
+	public function isHTTPURL($url) {
+		return stripos($url, 'https://') === 0 || stripos($url, 'http://') === 0;
+	}
+
+	/**
+	 * Returns the last HTTP or HTTPS site the request has been redirected too using the Location HTTP header
+	 * This is a very ugly workaround about the missing functionality to restrict fopen() to protocols
+	 * @param string $location Needs to be a HTTPS or HTTP URL
+	 * @throws \Exception In case the initial URL is not a HTTP or HTTPS one
+	 * @return string
+	 */
+	public function getFinalLocationOfURL($location) {
+		if(!$this->isHTTPURL($location)) {
+			throw new \Exception('URL must begin with HTTPS or HTTP.');
+		}
+		$headerArray = $this->getHeaders($location, 1);
+
+		if($headerArray !== false && isset($headerArray['Location'])) {
+			while($this->isHTTPURL($headerArray['Location'])) {
+				$location = $headerArray['Location'];
+				$headerArray = $this->getHeaders($location);
+			}
+		}
+
+		return $location;
+	}
+
+}
diff --git a/lib/private/server.php b/lib/private/server.php
index 912d5c4f635..7fa06298b29 100644
--- a/lib/private/server.php
+++ b/lib/private/server.php
@@ -212,6 +212,10 @@ class Server extends SimpleContainer implements IServerContainer {
 		$this->registerService('Db', function ($c) {
 			return new Db();
 		});
+		$this->registerService('HTTPHelper', function (SimpleContainer $c) {
+			$config = $c->query('AllConfig');
+			return new HTTPHelper($config);
+		});
 	}
 
 	/**
@@ -502,6 +506,14 @@ class Server extends SimpleContainer implements IServerContainer {
 		return $this->query('Db');
 	}
 
+	/**
+	 * Returns an instance of the HTTP helper class
+	 * @return \OC\HTTPHelper
+	 */
+	function getHTTPHelper() {
+		return $this->query('HTTPHelper');
+	}
+
 	/**
 	 * Get the certificate manager for the user
 	 *
diff --git a/lib/private/user/http.php b/lib/private/user/http.php
index 2bb8b4c864a..617e8adb3f2 100644
--- a/lib/private/user/http.php
+++ b/lib/private/user/http.php
@@ -72,6 +72,8 @@ class OC_User_HTTP extends OC_User_Backend {
 		curl_setopt($ch, CURLOPT_URL, $url);
 		curl_setopt($ch, CURLOPT_USERPWD, $user.':'.$password);
 		curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
+		curl_setopt($ch, CURLOPT_PROTOCOLS,  CURLPROTO_HTTP | CURLPROTO_HTTPS);
+		curl_setopt($ch, CURLOPT_REDIR_PROTOCOLS,  CURLPROTO_HTTP | CURLPROTO_HTTPS);
 
 		curl_exec($ch);
 
diff --git a/lib/private/util.php b/lib/private/util.php
index c5483c1654b..f84bbede0ac 100755
--- a/lib/private/util.php
+++ b/lib/private/util.php
@@ -5,8 +5,6 @@
  *
  */
 class OC_Util {
-	const USER_AGENT = 'ownCloud Server Crawler';
-	
 	public static $scripts = array();
 	public static $styles = array();
 	public static $headers = array();
@@ -1199,106 +1197,20 @@ class OC_Util {
 	}
 
 	/**
-	 * @Brief Get file content via curl.
+	 * Get URL content
 	 * @param string $url Url to get content
+	 * @deprecated Use \OC::$server->getHTTPHelper()->getUrlContent($url);
 	 * @throws Exception If the URL does not start with http:// or https://
 	 * @return string of the response or false on error
 	 * This function get the content of a page via curl, if curl is enabled.
 	 * If not, file_get_contents is used.
 	 */
 	public static function getUrlContent($url) {
-		if (strpos($url, 'http://') !== 0 && strpos($url, 'https://') !== 0) {
-			throw new Exception('$url must start with https:// or http://', 1);
-		}
-
-		if (function_exists('curl_init')) {
-			$curl = curl_init();
-			$max_redirects = 10;
-
-			curl_setopt($curl, CURLOPT_HEADER, 0);
-			curl_setopt($curl, CURLOPT_RETURNTRANSFER, 1);
-			curl_setopt($curl, CURLOPT_CONNECTTIMEOUT, 10);
-			curl_setopt($curl, CURLOPT_URL, $url);
-
-
-			curl_setopt($curl, CURLOPT_USERAGENT, self::USER_AGENT);
-			if (OC_Config::getValue('proxy', '') != '') {
-				curl_setopt($curl, CURLOPT_PROXY, OC_Config::getValue('proxy'));
-			}
-			if (OC_Config::getValue('proxyuserpwd', '') != '') {
-				curl_setopt($curl, CURLOPT_PROXYUSERPWD, OC_Config::getValue('proxyuserpwd'));
-			}
-
-			if (ini_get('open_basedir') === '' && ini_get('safe_mode') === 'Off') {
-				curl_setopt($curl, CURLOPT_FOLLOWLOCATION, true);
-				curl_setopt($curl, CURLOPT_MAXREDIRS, $max_redirects);
-				$data = curl_exec($curl);
-			} else {
-				curl_setopt($curl, CURLOPT_FOLLOWLOCATION, false);
-				$mr = $max_redirects;
-				if ($mr > 0) {
-					$newURL = curl_getinfo($curl, CURLINFO_EFFECTIVE_URL);
-					$rcurl = curl_copy_handle($curl);
-					curl_setopt($rcurl, CURLOPT_HEADER, true);
-					curl_setopt($rcurl, CURLOPT_NOBODY, true);
-					curl_setopt($rcurl, CURLOPT_FORBID_REUSE, false);
-					curl_setopt($rcurl, CURLOPT_RETURNTRANSFER, true);
-					curl_setopt($rcurl, CURLOPT_USERAGENT, self::USER_AGENT);
-					do {
-						curl_setopt($rcurl, CURLOPT_URL, $newURL);
-						$header = curl_exec($rcurl);
-						if (curl_errno($rcurl)) {
-							$code = 0;
-						} else {
-							$code = curl_getinfo($rcurl, CURLINFO_HTTP_CODE);
-							if ($code == 301 || $code == 302) {
-								preg_match('/Location:(.*?)\n/', $header, $matches);
-								$newURL = trim(array_pop($matches));
-							} else {
-								$code = 0;
-							}
-						}
-					} while ($code && --$mr);
-					curl_close($rcurl);
-					if ($mr > 0) {
-						curl_setopt($curl, CURLOPT_URL, $newURL);
-					}
-				}
-
-				if ($mr == 0 && $max_redirects > 0) {
-					$data = false;
-				} else {
-					$data = curl_exec($curl);
-				}
-			}
-			curl_close($curl);
-		} else {
-			$contextArray = null;
-
-			if (OC_Config::getValue('proxy', '') != '') {
-				$contextArray = array(
-					'http' => array(
-						'header' => 'User-Agent: ' . self::USER_AGENT . "\r\n",
-						'timeout' => 10,
-						'proxy' => OC_Config::getValue('proxy')
-					)
-				);
-			} else {
-				$contextArray = array(
-					'http' => array(
-						'header' => 'User-Agent: ' . self::USER_AGENT . "\r\n",
-						'timeout' => 10
-					)
-				);
-			}
-
-			$ctx = stream_context_create(
-				$contextArray
-			);
-			$data = @file_get_contents($url, 0, $ctx);
-
+		try {
+			return \OC::$server->getHTTPHelper()->getUrlContent($url);
+		} catch (\Exception $e) {
+			throw $e;
 		}
-		return $data;
 	}
 
 	/**
diff --git a/lib/public/iservercontainer.php b/lib/public/iservercontainer.php
index 1abf0d9938d..a093ff3a640 100644
--- a/lib/public/iservercontainer.php
+++ b/lib/public/iservercontainer.php
@@ -242,4 +242,10 @@ interface IServerContainer {
 	 * @return \OCP\IEventSource
 	 */
 	function createEventSource();
+
+	/**
+	 * Returns an instance of the HTTP helper class
+	 * @return \OC\HTTPHelper
+	 */
+	function getHTTPHelper();
 }
diff --git a/tests/lib/httphelper.php b/tests/lib/httphelper.php
new file mode 100644
index 00000000000..191200aee3d
--- /dev/null
+++ b/tests/lib/httphelper.php
@@ -0,0 +1,88 @@
+<?php
+/**
+ * Copyright (c) 2014 Lukas Reschke <lukas@owncloud.com>
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+class TestHTTPHelper extends \PHPUnit_Framework_TestCase {
+
+	/** @var \OC\AllConfig*/
+	private $config;
+	/** @var \OC\HTTPHelper */
+	private $httpHelperMock;
+
+	function setUp() {
+		$this->config = $this->getMockBuilder('\OC\AllConfig')
+			->disableOriginalConstructor()->getMock();
+		$this->httpHelperMock = $this->getMockBuilder('\OC\HTTPHelper')
+			->setConstructorArgs(array($this->config))
+			->setMethods(array('getHeaders'))
+			->getMock();
+	}
+
+	public function testIsHTTPProvider() {
+		return array(
+			array('http://wwww.owncloud.org/enterprise/', true),
+			array('https://wwww.owncloud.org/enterprise/', true),
+			array('HTTPS://WWW.OWNCLOUD.ORG', true),
+			array('HTTP://WWW.OWNCLOUD.ORG', true),
+			array('FILE://WWW.OWNCLOUD.ORG', false),
+			array('file://www.owncloud.org', false),
+			array('FTP://WWW.OWNCLOUD.ORG', false),
+			array('ftp://www.owncloud.org', false),
+		);
+	}
+
+	/**
+	 * Note: Not using a dataprovider because onConsecutiveCalls expects not
+	 * an array but the function arguments directly
+	 */
+	public function testGetFinalLocationOfURLValid() {
+		$url = 'https://www.owncloud.org/enterprise/';
+		$expected = 'https://www.owncloud.com/enterprise/';
+		$this->httpHelperMock->expects($this->any())
+			->method('getHeaders')
+			->will($this->onConsecutiveCalls(
+				array('Location' => 'http://www.owncloud.com/enterprise/'),
+				array('Location' => 'https://www.owncloud.com/enterprise/')
+			));
+		$result = $this->httpHelperMock->getFinalLocationOfURL($url);
+		$this->assertSame($expected, $result);
+	}
+
+	/**
+	 * Note: Not using a dataprovider because onConsecutiveCalls expects not
+	 * an array but the function arguments directly
+	 */
+	public function testGetFinalLocationOfURLInvalid() {
+		$url = 'https://www.owncloud.org/enterprise/';
+		$expected = 'http://www.owncloud.com/enterprise/';
+		$this->httpHelperMock->expects($this->any())
+			->method('getHeaders')
+			->will($this->onConsecutiveCalls(
+				array('Location' => 'http://www.owncloud.com/enterprise/'),
+				array('Location' => 'file://etc/passwd'),
+				array('Location' => 'http://www.example.com/')
+			));
+		$result = $this->httpHelperMock->getFinalLocationOfURL($url);
+		$this->assertSame($expected, $result);
+	}
+
+	/**
+	 * @expectedException \Exception
+	 * @expectedExceptionMessage URL must begin with HTTPS or HTTP.
+	 */
+	public function testGetFinalLocationOfURLException() {
+		$this->httpHelperMock->getFinalLocationOfURL('file://etc/passwd');
+	}
+
+	/**
+	 * @dataProvider testIsHTTPProvider
+	 */
+	public function testIsHTTP($url, $expected) {
+			$this->assertSame($expected, $this->httpHelperMock->isHTTPURL($url));
+	}
+
+}