From 6eeb905871fc7a671f99fd22c2592358a6abc02d Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 11 Sep 2014 19:21:56 +0200 Subject: [PATCH] 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 --- apps/files/ajax/newfile.php | 9 +- lib/private/files/storage/dav.php | 4 + lib/private/httphelper.php | 177 ++++++++++++++++++++++++++++++ lib/private/server.php | 12 ++ lib/private/user/http.php | 2 + lib/private/util.php | 100 +---------------- lib/public/iservercontainer.php | 6 + tests/lib/httphelper.php | 88 +++++++++++++++ 8 files changed, 302 insertions(+), 96 deletions(-) create mode 100644 lib/private/httphelper.php create mode 100644 tests/lib/httphelper.php 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 @@ + + * 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 @@ + + * 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)); + } + +} -- 2.39.5