Issues in proxy.php - All Issues - Inspection of "Set current key for custom multi option fields for..." - SimpleMachines/SMF2.1 - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — release-2.1 (#4574)

by Matthew

created 2018-03-16 23:33 UTC

proxy.php (6 issues)

Labels

Severity

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?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
 */

if (!defined('SMF'))
	define('SMF', 'proxy');

global $proxyhousekeeping;
function myFunction($a, $b) {
    // Do something
}

/**
 * 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 $maxDays until enties get deleted */
	protected $maxDays;

	/**
	 * 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 . '/Subs.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';
		$this->maxDays = 5;
	}

	/**
	 * 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;

		// Basic sanity check
		$_GET['request'] = filter_var($_GET['request'], FILTER_VALIDATE_URL);

		// We aren't going anywhere without these
		if (empty($_GET['hash']) || empty($_GET['request']))
			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 = time();

		// Is the cache expired?
		if (!$cached || $time - $cached['time'] > ($this->maxDays * 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);
		$ext = strtolower(pathinfo(parse_url($request, PHP_URL_PATH), PATHINFO_EXTENSION));

		$image = fetch_web_data($request);

		// Looks like nobody was home
		if (empty($image))
			return -1;

		// What kind of file did they give us?
		$finfo = finfo_open(FILEINFO_MIME_TYPE);
		$mime_type = finfo_buffer($finfo, $image);

		// SVG needs a little extra care
		if ($ext == 'svg' && $mime_type == 'text/plain')
			$mime_type = 'image/svg+xml';

		// Make sure the url is returning an image
		if (strpos($mime_type, 'image/') !== 0)
			return -1;

		// Validate the filesize
		$size = strlen($image);
		if ($size > ($this->maxSize * 1024))
			return 0;

		// Cache it for later
		return file_put_contents($dest, json_encode(array(
			'content_type' => $mime_type,
			'size' => $size,
			'time' => time(),
			'body' => base64_encode($image),
		))) === 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;
	}

	/**
	 * Delete all old entries
	 *
	 * @access public
	 * @return void
	 */
	public function housekeeping()
	{
		$path = $this->cache . '/';
		if ($handle = opendir($path)) {

			while (false !== ($file = readdir($handle)))
			{
				$filelastmodified = filemtime($path . $file);

				if ((time() - $filelastmodified) > ($this->maxDays * 86400))
				{
				   unlink($path . $file);
				}

			}

			closedir($handle);
		}
	}
}

if (empty($proxyhousekeeping))
{
	$proxy = new ProxyServer();
	$proxy->serve();
}

?>

Pull Request — release-2.1 (#4574)

proxy.php (6 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

1. Pass all data via parameters

2. Create a class that maintains your state

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		if (!defined('SMF'))
17		define('SMF', 'proxy');
18
19		global $proxyhousekeeping;
		0 ignored issues – show Compatibility Best Practice introduced 2018-02-09 18:24 UTC by Report Bug Copy Issue Report Show Similar Issues like this 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...
20
21		/**
22		* Class ProxyServer
23		*/
24		class ProxyServer
25		{
26		/** @var bool $enabled Whether or not this is enabled */
27		protected $enabled;
28
29		/** @var int $maxSize The maximum size for files to cache */
30		protected $maxSize;
31
32		/** @var string $secret A secret code used for hashing */
33		protected $secret;
34
35		/** @var string The cache directory */
36		protected $cache;
37
38		/** @var int $maxDays until enties get deleted */
39		protected $maxDays;
40
41		/**
42		* Constructor, loads up the Settings for the proxy
43		*
44		* @access public
45		*/
46		public function __construct()
47		{
48		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 Show Similar Issues like this 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...
49
50		require_once(dirname(__FILE__) . '/Settings.php');
51		require_once($sourcedir . '/Subs.php');
52
53		// Turn off all error reporting; any extra junk makes for an invalid image.
54		error_reporting(0);
55
56		$this->enabled = (bool) $image_proxy_enabled;
57		$this->maxSize = (int) $image_proxy_maxsize;
58		$this->secret = (string) $image_proxy_secret;
59		$this->cache = $cachedir . '/images';
60		$this->maxDays = 5;
61		}
62
63		/**
64		* Checks whether the request is valid or not
65		*
66		* @access public
67		* @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 Show Similar Issues like this 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...
68		*/
69		public function checkRequest()
70		{
71		if (!$this->enabled)
72		return false;
73
74		// Try to create the image cache directory if it doesn't exist
75		if (!file_exists($this->cache))
76		if (!mkdir($this->cache) \|\| !copy(dirname($this->cache) . '/index.php', $this->cache . '/index.php'))
77		return false;
78
79		// Basic sanity check
80		$_GET['request'] = filter_var($_GET['request'], FILTER_VALIDATE_URL);
81
82		// We aren't going anywhere without these
83		if (empty($_GET['hash']) \|\| empty($_GET['request']))
84		return false;
85
86		$hash = $_GET['hash'];
87		$request = $_GET['request'];
88
89		if (md5($request . $this->secret) != $hash)
90		return false;
91
92		// Attempt to cache the request if it doesn't exist
93		if (!$this->isCached($request))
94		return $this->cacheImage($request);
95
96		return true;
97		}
98
99		/**
100		* Serves the request
101		*
102		* @access public
103		* @return void
104		*/
105		public function serve()
106		{
107		$request = $_GET['request'];
108		$cached_file = $this->getCachedPath($request);
109		$cached = json_decode(file_get_contents($cached_file), true);
110
111		// Did we get an error when trying to fetch the image
112		$response = $this->checkRequest();
113		if ($response === -1)
114		{
115		// Throw a 404
116		header('HTTP/1.0 404 Not Found');
117		exit;
118		}
119		// Right, image not cached? Simply redirect, then.
120		if ($response === 0)
121		{
122		$this::redirectexit($request);
123		}
124
125		$time = time();
126
127		// Is the cache expired?
128		if (!$cached \|\| $time - $cached['time'] > ($this->maxDays * 86400))
129		{
130		@unlink($cached_file);
		0 ignored issues – show Security Best Practice introduced 2016-12-27 02:48 UTC by Report Bug Copy Issue Report Show Similar Issues like this 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...
131		if ($this->checkRequest())
132		$this->serve();
133		$this::redirectexit($request);
134		}
135
136		$eTag = '"' . substr(sha1($request) . $cached['time'], 0, 64) . '"';
137	View Code Duplication	if (!empty($_SERVER['HTTP_IF_NONE_MATCH']) && strpos($_SERVER['HTTP_IF_NONE_MATCH'], $eTag) !== false)
138		{
139		header('HTTP/1.1 304 Not Modified');
140		exit;
141		}
142
143		// Make sure we're serving an image
144		$contentParts = explode('/', !empty($cached['content_type']) ? $cached['content_type'] : '');
145		if ($contentParts[0] != 'image')
146		exit;
147
148		$max_age = $time - $cached['time'] + (5 * 86400);
149		header('content-type: ' . $cached['content_type']);
150		header('content-length: ' . $cached['size']);
151		header('cache-control: public, max-age=' . $max_age );
152		header('last-modified: ' . gmdate('D, d M Y H:i:s', $cached['time']) . ' GMT');
153		header('etag: ' . $eTag);
154		echo base64_decode($cached['body']);
155		}
156
157		/**
158		* Returns the request's hashed filepath
159		*
160		* @access public
161		* @param string $request The request to get the path for
162		* @return string The hashed filepath for the specified request
163		*/
164		protected function getCachedPath($request)
165		{
166		return $this->cache . '/' . sha1($request . $this->secret);
167		}
168
169		/**
170		* Check whether the image exists in local cache or not
171		*
172		* @access protected
173		* @param string $request The image to check for in the cache
174		* @return bool Whether or not the requested image is cached
175		*/
176		protected function isCached($request)
177		{
178		return file_exists($this->getCachedPath($request));
179		}
180
181		/**
182		* Attempts to cache the image while validating it
183		*
184		* @access protected
185		* @param string $request The image to cache/validate
186		* @return int -1 error, 0 too big, 1 valid image
187		*/
188		protected function cacheImage($request)
189		{
190		$dest = $this->getCachedPath($request);
191		$ext = strtolower(pathinfo(parse_url($request, PHP_URL_PATH), PATHINFO_EXTENSION));
192
193		$image = fetch_web_data($request);
194
195		// Looks like nobody was home
196		if (empty($image))
197		return -1;
198
199		// What kind of file did they give us?
200		$finfo = finfo_open(FILEINFO_MIME_TYPE);
201		$mime_type = finfo_buffer($finfo, $image);
202
203		// SVG needs a little extra care
204		if ($ext == 'svg' && $mime_type == 'text/plain')
205		$mime_type = 'image/svg+xml';
206
207		// Make sure the url is returning an image
208		if (strpos($mime_type, 'image/') !== 0)
209		return -1;
210
211		// Validate the filesize
212		$size = strlen($image);
213		if ($size > ($this->maxSize * 1024))
214		return 0;
215
216		// Cache it for later
217		return file_put_contents($dest, json_encode(array(
218		'content_type' => $mime_type,
219		'size' => $size,
220		'time' => time(),
221		'body' => base64_encode($image),
222		))) === false ? -1 : 1;
223		}
224
225		/**
226		* Static helper function to redirect a request
227		*
228		* @access public
229		* @param type $request
230		* @return void
231		*/
232		static public function redirectexit($request)
		0 ignored issues – show Coding Style introduced 2018-02-16 12:33 UTC by Report Bug Copy Issue Report Show Similar Issues like this As per PSR2, the `static` declaration should come after the visibility declaration. Loading history...
233		{
234		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 Show Similar Issues like this `'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 107 `$request` is passed to ProxyServer::redirectexit() in proxy.php on line 122 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...
235		exit;
236		}
237
238		/**
239		* Delete all old entries
240		*
241		* @access public
242		* @return void
243		*/
244		public function housekeeping()
245		{
246		$path = $this->cache . '/';
247		if ($handle = opendir($path)) {
248
249		while (false !== ($file = readdir($handle)))
250		{
251		$filelastmodified = filemtime($path . $file);
252
253		if ((time() - $filelastmodified) > ($this->maxDays * 86400))
254		{
255		unlink($path . $file);
256		}
257
258		}
259
260		closedir($handle);
261		}
262		}
263		}
264
265		if (empty($proxyhousekeeping))
266		{
267		$proxy = new ProxyServer();
268		$proxy->serve();
269		}
270
271		?>

SimpleMachines / SMF2.1

Pull Request — release-2.1 (#4574)

proxy.php (6 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

1. Pass all data via parameters

2. Create a class that maintains your state

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