[Pkg-owncloud-commits] [owncloud] 131/258: Do only follow HTTP and HTTPS redirects
David Prévot
taffit at moszumanska.debian.org
Sat Oct 11 17:22:29 UTC 2014
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch master
in repository owncloud.
commit cb3bc5ad31bac636ae38a46f1f2b178709ffe5d6
Author: Lukas Reschke <lukas at owncloud.com>
Date: Thu Sep 11 19:21:56 2014 +0200
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
Conflicts:
apps/files/ajax/newfile.php
lib/private/files/storage/dav.php
lib/private/server.php
lib/private/util.php
lib/public/iservercontainer.php
---
apps/files/ajax/newfile.php | 8 +-
lib/private/files/storage/dav.php | 4 +
lib/private/httphelper.php | 177 ++++++++++++++++++++++++++++++++++++++
lib/private/server.php | 13 +++
lib/private/user/http.php | 2 +
lib/private/util.php | 95 ++------------------
lib/public/iservercontainer.php | 5 ++
tests/lib/httphelper.php | 88 +++++++++++++++++++
8 files changed, 301 insertions(+), 91 deletions(-)
diff --git a/apps/files/ajax/newfile.php b/apps/files/ajax/newfile.php
index 9cfe51a..1b97178 100644
--- a/apps/files/ajax/newfile.php
+++ b/apps/files/ajax/newfile.php
@@ -93,7 +93,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 +105,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 726688f..0ba69fd 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);
@@ -281,6 +283,8 @@ class DAV extends \OC\Files\Storage\Common {
curl_setopt($curl, CURLOPT_INFILE, $source); // file pointer
curl_setopt($curl, CURLOPT_INFILESIZE, filesize($path));
curl_setopt($curl, CURLOPT_PUT, 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 0000000..8b7aebb
--- /dev/null
+++ b/lib/private/httphelper.php
@@ -0,0 +1,177 @@
+<?php
+/**
+ * Copyright (c) 2014 Lukas Reschke <lukas at 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 d1c5768..81c2283 100644
--- a/lib/private/server.php
+++ b/lib/private/server.php
@@ -202,6 +202,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);
+ });
}
/**
@@ -468,4 +472,13 @@ class Server extends SimpleContainer implements IServerContainer {
function getDb() {
return $this->query('Db');
}
+
+ /**
+ * Returns an instance of the HTTP helper class
+ * @return \OC\HTTPHelper
+ */
+ function getHTTPHelper() {
+ return $this->query('HTTPHelper');
+ }
+
}
diff --git a/lib/private/user/http.php b/lib/private/user/http.php
index 2bb8b4c..617e8ad 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 0d6e11d..3e1f334 100755
--- a/lib/private/util.php
+++ b/lib/private/util.php
@@ -1233,103 +1233,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, "ownCloud Server Crawler");
- 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);
- 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(
- 'timeout' => 10,
- 'proxy' => OC_Config::getValue('proxy')
- )
- );
- } else {
- $contextArray = array(
- 'http' => array(
- '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 c0413c1..d1d6487 100644
--- a/lib/public/iservercontainer.php
+++ b/lib/public/iservercontainer.php
@@ -227,4 +227,9 @@ interface IServerContainer {
*/
function getSearch();
+ /**
+ * 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 0000000..191200a
--- /dev/null
+++ b/tests/lib/httphelper.php
@@ -0,0 +1,88 @@
+<?php
+/**
+ * Copyright (c) 2014 Lukas Reschke <lukas at 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));
+ }
+
+}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git
More information about the Pkg-owncloud-commits
mailing list