ProxyServer::redirectexit() - Code Metrics - Inspection of "overhaul of proxy.php (#4550)" - SimpleMachines/SMF2.1 - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — release-2.1 ( 62bce6...b11117 )

by Colin

created 2018-03-08 07:55 UTC

ProxyServer::redirectexit() A

↳ Parent: ProxyServer

Complexity

Conditions	1
Paths	1

Size

Total Lines	5
Code Lines	3

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	1
eloc	3
nc	1
nop	1
dl	0
loc	5
rs	9.4285
c	0
b	0
f	0

<?php

/**
 * This is a lightweight proxy for serving images, generally meant to be used alongside SSL
 *
 * Simple Machines Forum (SMF)
 *
 * @package SMF
 * @author Simple Machines http://www.simplemachines.org
 * @copyright 2018 Simple Machines and individual contributors
 * @license http://www.simplemachines.org/about/smf/license.php BSD
 *
 * @version 2.1 Beta 4
 */

define('SMF', 'proxy');

/**
 * Class ProxyServer
 */
class ProxyServer
{
	/** @var bool $enabled Whether or not this is enabled */
	protected $enabled;

	/** @var int $maxSize The maximum size for files to cache */
	protected $maxSize;

	/** @var string $secret A secret code used for hashing */
	protected $secret;

	/** @var string The cache directory */
	protected $cache;

	/** @var int time() value */
	protected $time;

	/**
	 * Constructor, loads up the Settings for the proxy
	 *
	 * @access public
	 */
	public function __construct()
	{
		global $image_proxy_enabled, $image_proxy_maxsize, $image_proxy_secret, $cachedir, $sourcedir;
function myFunction($a, $b) {
    // Do something
}

		require_once(dirname(__FILE__) . '/Settings.php');
		require_once($sourcedir . '/Class-CurlFetchWeb.php');

		// Turn off all error reporting; any extra junk makes for an invalid image.
		error_reporting(0);

		$this->enabled = (bool) $image_proxy_enabled;
		$this->maxSize = (int) $image_proxy_maxsize;
		$this->secret = (string) $image_proxy_secret;
		$this->cache = $cachedir . '/images';
	}

	/**
	 * Checks whether the request is valid or not
	 *
	 * @access public
	 * @return bool Whether the request is valid

	 */
	public function checkRequest()
	{
		if (!$this->enabled)
			return false;

		// Try to create the image cache directory if it doesn't exist
		if (!file_exists($this->cache))
			if (!mkdir($this->cache) || !copy(dirname($this->cache) . '/index.php', $this->cache . '/index.php'))
				return false;

		if (empty($_GET['hash']) || empty($_GET['request']) || ($_GET['request'] === "http:") || ($_GET['request'] === "https:"))
<?php

$singleQuoted = 'Value';
$doubleQuoted = "\tSingle is $singleQuoted";

print $doubleQuoted;
			return false;

		$hash = $_GET['hash'];
		$request = $_GET['request'];

		if (md5($request . $this->secret) != $hash)
			return false;

		// Attempt to cache the request if it doesn't exist
		if (!$this->isCached($request))
			return $this->cacheImage($request);

		return true;
	}

	/**
	 * Serves the request
	 *
	 * @access public
	 * @return void
	 */
	public function serve()
	{
		$request = $_GET['request'];
		$cached_file = $this->getCachedPath($request);
		$cached = json_decode(file_get_contents($cached_file), true);

		// Did we get an error when trying to fetch the image
		$response = $this->checkRequest();
		if ($response === -1)
		{
			// Throw a 404
			header('HTTP/1.0 404 Not Found');
			exit;
		}
		// Right, image not cached? Simply redirect, then.
		if ($response === 0)
		{
			$this::redirectexit($request);
		}

		$time = $this->getTime();

		// Is the cache expired?
		if (!$cached || $time - $cached['time'] > (5 * 86400))
		{
			@unlink($cached_file);
// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
			if ($this->checkRequest())
				$this->serve();
			$this::redirectexit($request);
		}

		$eTag = '"' . substr(sha1($request) . $cached['time'], 0, 64) . '"';
		if (!empty($_SERVER['HTTP_IF_NONE_MATCH']) && strpos($_SERVER['HTTP_IF_NONE_MATCH'], $eTag) !== false)
		{
			header('HTTP/1.1 304 Not Modified');
			exit;
		}

		// Make sure we're serving an image
		$contentParts = explode('/', !empty($cached['content_type']) ? $cached['content_type'] : '');
		if ($contentParts[0] != 'image')
			exit;

		$max_age = $time - $cached['time'] + (5 * 86400);
		header('content-type: ' . $cached['content_type']);
		header('content-length: ' . $cached['size']);
		header('cache-control: public, max-age=' . $max_age );
		header('last-modified: ' . gmdate('D, d M Y H:i:s', $cached['time']) . ' GMT');
		header('etag: ' . $eTag);
		echo base64_decode($cached['body']);
	}

	/**
	 * Returns the request's hashed filepath
	 *
	 * @access public
	 * @param string $request The request to get the path for
	 * @return string The hashed filepath for the specified request
	 */
	protected function getCachedPath($request)
	{
		return $this->cache . '/' . sha1($request . $this->secret);
	}

	/**
	 * Check whether the image exists in local cache or not
	 *
	 * @access protected
	 * @param string $request The image to check for in the cache
	 * @return bool Whether or not the requested image is cached
	 */
	protected function isCached($request)
	{
		return file_exists($this->getCachedPath($request));
	}

	/**
	 * Attempts to cache the image while validating it
	 *
	 * @access protected
	 * @param string $request The image to cache/validate
	 * @return int -1 error, 0 too big, 1 valid image
	 */
	protected function cacheImage($request)
	{
		$dest = $this->getCachedPath($request);
		$curl = new curl_fetch_web_data(array(CURLOPT_BINARYTRANSFER => 1));
		$curl_request = $curl->get_url_data($request);
		$responseCode = $curl_request->result('code');
		$response = $curl_request->result();

		if (empty($response) || $responseCode != 200)
		{
			return -1;
		}

		$headers = $response['headers'];

		// Make sure the url is returning an image
		$contentParts = explode('/', !empty($headers['content-type']) ? $headers['content-type'] : '');
		if ($contentParts[0] != 'image')
			return -1;

		// Validate the filesize
		if ($response['size'] > ($this->maxSize * 1024))
			return 0;

		$time = $this->getTime();

		return file_put_contents($dest, json_encode(array(
			'content_type' => $headers['content-type'],
			'size' => $response['size'],
			'time' => $time,
			'body' => base64_encode($response['body']),
		))) === false ? -1 : 1; 
	}

	/**
	 * Static helper function to redirect a request
	 * 
	 * @access public
	 * @param type $request
	 * @return void
	 */
	static public function redirectexit($request)

	{
		header('Location: ' . $request, false, 301);
if ( ! in_array($value, array('this-is-allowed', 'and-this-too'), true)) {
    throw new \InvalidArgumentException('This input is not allowed.');
}
		exit;
	}

	/**
	 * Helper function to call time() once with the right logic
	 * 
	 * @return int
	 */
	protected function getTime()
	{
		if (empty($this->time))
		{
			$old_timezone = date_default_timezone_get();
			date_default_timezone_set('GMT');
			$this->time = time();
			date_default_timezone_set($old_timezone);
		}

		return $this->time;
	}
}

$proxy = new ProxyServer();
$proxy->serve();

?>

Push — release-2.1 ( 62bce6...b11117 )

ProxyServer::redirectexit() A

Complexity

Size

Duplication

Importance

1. Pass all data via parameters

2. Create a class that maintains your state

1 path for user data to reach this point

Response Splitting Attacks

General Strategies to prevent injection

1		<?php
2
3		/**
4		* This is a lightweight proxy for serving images, generally meant to be used alongside SSL
5		*
6		* Simple Machines Forum (SMF)
7		*
8		* @package SMF
9		* @author Simple Machines http://www.simplemachines.org
10		* @copyright 2018 Simple Machines and individual contributors
11		* @license http://www.simplemachines.org/about/smf/license.php BSD
12		*
13		* @version 2.1 Beta 4
14		*/
15
16		define('SMF', 'proxy');
17
18		/**
19		* Class ProxyServer
20		*/
21		class ProxyServer
22		{
23		/** @var bool $enabled Whether or not this is enabled */
24		protected $enabled;
25
26		/** @var int $maxSize The maximum size for files to cache */
27		protected $maxSize;
28
29		/** @var string $secret A secret code used for hashing */
30		protected $secret;
31
32		/** @var string The cache directory */
33		protected $cache;
34
35		/** @var int time() value */
36		protected $time;
37
38		/**
39		* Constructor, loads up the Settings for the proxy
40		*
41		* @access public
42		*/
43		public function __construct()
44		{
45		global $image_proxy_enabled, $image_proxy_maxsize, $image_proxy_secret, $cachedir, $sourcedir;
		0 ignored issues – show Compatibility Best Practice introduced 2017-08-25 21:51 UTC by Report Bug Copy Issue Report Use of `global` functionality is not recommended; it makes your code harder to test, and less reusable. Instead of relying on `global` state, we recommend one of these alternatives: 1. Pass all data via parameters function myFunction($a, $b) { // Do something } 2. Create a class that maintains your state class MyClass { private $a; private $b; public function __construct($a, $b) { $this->a = $a; $this->b = $b; } public function myFunction() { // Do something } } Loading history...
46
47		require_once(dirname(__FILE__) . '/Settings.php');
48		require_once($sourcedir . '/Class-CurlFetchWeb.php');
49
50		// Turn off all error reporting; any extra junk makes for an invalid image.
51		error_reporting(0);
52
53		$this->enabled = (bool) $image_proxy_enabled;
54		$this->maxSize = (int) $image_proxy_maxsize;
55		$this->secret = (string) $image_proxy_secret;
56		$this->cache = $cachedir . '/images';
57		}
58
59		/**
60		* Checks whether the request is valid or not
61		*
62		* @access public
63		* @return bool Whether the request is valid
		0 ignored issues – show Documentation introduced 2018-02-16 12:33 UTC by Report Bug Copy Issue Report Should the return type not be `integer\|boolean`? This check compares the return type specified in the `@return` annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch. Loading history...
64		*/
65		public function checkRequest()
66		{
67		if (!$this->enabled)
68		return false;
69
70		// Try to create the image cache directory if it doesn't exist
71		if (!file_exists($this->cache))
72		if (!mkdir($this->cache) \|\| !copy(dirname($this->cache) . '/index.php', $this->cache . '/index.php'))
73		return false;
74
75		if (empty($_GET['hash']) \|\| empty($_GET['request']) \|\| ($_GET['request'] === "http:") \|\| ($_GET['request'] === "https:"))
		0 ignored issues – show Coding Style Comprehensibility introduced 2018-02-16 12:33 UTC by Report Bug Copy Issue Report The string literal `http:` does not require double quotes, as per coding-style, please use single quotes. PHP provides two ways to mark string literals. Either with single quotes `'literal'` or with double quotes `"literal"`. The difference between these is that string literals in double quotes may contain variables with are evaluated at run-time as well as escape sequences. String literals in single quotes on the other hand are evaluated very literally and the only two characters that needs escaping in the literal are the single quote itself (`\'`) and the backslash (`\\`). Every other character is displayed as is. Double quoted string literals may contain other variables or more complex escape sequences. <?php $singleQuoted = 'Value'; $doubleQuoted = "\tSingle is $singleQuoted"; print $doubleQuoted; will print an indented: `Single is Value` If your string literal does not contain variables or escape sequences, it should be defined using single quotes to make that fact clear. For more information on PHP string literals and available escape sequences see the PHP core documentation. Loading history... Coding Style Comprehensibility introduced 2018-02-16 12:33 UTC by Report Bug Copy Issue Report The string literal `https:` does not require double quotes, as per coding-style, please use single quotes. PHP provides two ways to mark string literals. Either with single quotes `'literal'` or with double quotes `"literal"`. The difference between these is that string literals in double quotes may contain variables with are evaluated at run-time as well as escape sequences. String literals in single quotes on the other hand are evaluated very literally and the only two characters that needs escaping in the literal are the single quote itself (`\'`) and the backslash (`\\`). Every other character is displayed as is. Double quoted string literals may contain other variables or more complex escape sequences. <?php $singleQuoted = 'Value'; $doubleQuoted = "\tSingle is $singleQuoted"; print $doubleQuoted; will print an indented: `Single is Value` If your string literal does not contain variables or escape sequences, it should be defined using single quotes to make that fact clear. For more information on PHP string literals and available escape sequences see the PHP core documentation. Loading history...
76		return false;
77
78		$hash = $_GET['hash'];
79		$request = $_GET['request'];
80
81		if (md5($request . $this->secret) != $hash)
82		return false;
83
84		// Attempt to cache the request if it doesn't exist
85		if (!$this->isCached($request))
86		return $this->cacheImage($request);
87
88		return true;
89		}
90
91		/**
92		* Serves the request
93		*
94		* @access public
95		* @return void
96		*/
97		public function serve()
98		{
99		$request = $_GET['request'];
100		$cached_file = $this->getCachedPath($request);
101		$cached = json_decode(file_get_contents($cached_file), true);
102
103		// Did we get an error when trying to fetch the image
104		$response = $this->checkRequest();
105		if ($response === -1)
106		{
107		// Throw a 404
108		header('HTTP/1.0 404 Not Found');
109		exit;
110		}
111		// Right, image not cached? Simply redirect, then.
112		if ($response === 0)
113		{
114		$this::redirectexit($request);
115		}
116
117		$time = $this->getTime();
118
119		// Is the cache expired?
120		if (!$cached \|\| $time - $cached['time'] > (5 * 86400))
121		{
122		@unlink($cached_file);
		0 ignored issues – show Security Best Practice introduced 2016-12-27 02:48 UTC by Report Bug Copy Issue Report It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended. If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of @mkdir($dir); // Better use if (@mkdir($dir) === false) { throw new \RuntimeException('The directory '.$dir.' could not be created.'); } Loading history...
123		if ($this->checkRequest())
124		$this->serve();
125		$this::redirectexit($request);
126		}
127
128		$eTag = '"' . substr(sha1($request) . $cached['time'], 0, 64) . '"';
129	View Code Duplication	if (!empty($_SERVER['HTTP_IF_NONE_MATCH']) && strpos($_SERVER['HTTP_IF_NONE_MATCH'], $eTag) !== false)
130		{
131		header('HTTP/1.1 304 Not Modified');
132		exit;
133		}
134
135		// Make sure we're serving an image
136		$contentParts = explode('/', !empty($cached['content_type']) ? $cached['content_type'] : '');
137		if ($contentParts[0] != 'image')
138		exit;
139
140		$max_age = $time - $cached['time'] + (5 * 86400);
141		header('content-type: ' . $cached['content_type']);
142		header('content-length: ' . $cached['size']);
143		header('cache-control: public, max-age=' . $max_age );
144		header('last-modified: ' . gmdate('D, d M Y H:i:s', $cached['time']) . ' GMT');
145		header('etag: ' . $eTag);
146		echo base64_decode($cached['body']);
147		}
148
149		/**
150		* Returns the request's hashed filepath
151		*
152		* @access public
153		* @param string $request The request to get the path for
154		* @return string The hashed filepath for the specified request
155		*/
156		protected function getCachedPath($request)
157		{
158		return $this->cache . '/' . sha1($request . $this->secret);
159		}
160
161		/**
162		* Check whether the image exists in local cache or not
163		*
164		* @access protected
165		* @param string $request The image to check for in the cache
166		* @return bool Whether or not the requested image is cached
167		*/
168		protected function isCached($request)
169		{
170		return file_exists($this->getCachedPath($request));
171		}
172
173		/**
174		* Attempts to cache the image while validating it
175		*
176		* @access protected
177		* @param string $request The image to cache/validate
178		* @return int -1 error, 0 too big, 1 valid image
179		*/
180		protected function cacheImage($request)
181		{
182		$dest = $this->getCachedPath($request);
183		$curl = new curl_fetch_web_data(array(CURLOPT_BINARYTRANSFER => 1));
184		$curl_request = $curl->get_url_data($request);
185		$responseCode = $curl_request->result('code');
186		$response = $curl_request->result();
187
188		if (empty($response) \|\| $responseCode != 200)
189		{
190		return -1;
191		}
192
193		$headers = $response['headers'];
194
195		// Make sure the url is returning an image
196		$contentParts = explode('/', !empty($headers['content-type']) ? $headers['content-type'] : '');
197		if ($contentParts[0] != 'image')
198		return -1;
199
200		// Validate the filesize
201		if ($response['size'] > ($this->maxSize * 1024))
202		return 0;
203
204		$time = $this->getTime();
205
206		return file_put_contents($dest, json_encode(array(
207		'content_type' => $headers['content-type'],
208		'size' => $response['size'],
209		'time' => $time,
210		'body' => base64_encode($response['body']),
211		))) === false ? -1 : 1;
212		}
213
214		/**
215		* Static helper function to redirect a request
216		*
217		* @access public
218		* @param type $request
219		* @return void
220		*/
221		static public function redirectexit($request)
		0 ignored issues – show Coding Style introduced 2018-02-16 12:33 UTC by Report Bug Copy Issue Report As per PSR2, the `static` declaration should come after the visibility declaration. Loading history...
222		{
223		header('Location: ' . $request, false, 301);
		0 ignored issues – show Security Response Splitting introduced 2018-02-16 12:33 UTC by Report Bug Copy Issue Report `'Location: ' . $request` can contain request data and is used in response header context(s) leading to a potential security vulnerability. 1 path for user data to reach this point Read from `$_GET,` and `$request` is assigned in proxy.php on line 99 `$request` is passed to ProxyServer::redirectexit() in proxy.php on line 114 Response Splitting Attacks Allowing an attacker to set a response header, opens your application to response splitting attacks; effectively allowing an attacker to send any response, he would like. General Strategies to prevent injection In general, it is advisable to prevent any user-data to reach this point. This can be done by white-listing certain values: if ( ! in_array($value, array('this-is-allowed', 'and-this-too'), true)) { throw new \InvalidArgumentException('This input is not allowed.'); } For numeric data, we recommend to explicitly cast the data: $sanitized = (integer) $tainted; Loading history...
224		exit;
225		}
226
227		/**
228		* Helper function to call time() once with the right logic
229		*
230		* @return int
231		*/
232		protected function getTime()
233		{
234		if (empty($this->time))
235		{
236		$old_timezone = date_default_timezone_get();
237		date_default_timezone_set('GMT');
238		$this->time = time();
239		date_default_timezone_set($old_timezone);
240		}
241
242		return $this->time;
243		}
244		}
245
246		$proxy = new ProxyServer();
247		$proxy->serve();
248
249		?>

SimpleMachines / SMF2.1

Push — release-2.1 ( 62bce6...b11117 )

ProxyServer::redirectexit() A

Complexity

Size

Duplication

Importance

1. Pass all data via parameters

2. Create a class that maintains your state

1 path for user data to reach this point

Response Splitting Attacks

General Strategies to prevent injection

Duplication Side-by-Side

Filter issues like