Security - Code Metrics - Inspection of "Merge pull request #535 from trabisdementia/master" - XOOPS/XoopsCore - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( c622f7...33a06a )

by Richard

created 2017-03-12 20:08 UTC

Security B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	219
Duplicated Lines	0 %

Coupling/Cohesion

Components	1
Dependencies	5

Test Coverage

Coverage

97.47%

Importance

Changes

Metric	Value
dl	0
loc	219
ccs	77
cts	79
cp	0.9747
rs	8.6
c	0
b	0
f	0
wmc	37
lcom	1
cbo	5

11 Methods

Rating	Name	Size	Complexity
A	check()	4	1
C	validateToken()	39	11
A	clearTokens()	4	1
A	filterToken()	4	2
A	garbageCollection()	7	3
A	checkReferer()	14	4
A	getTokenHTML()	5	1
A	setErrors()	4	1
A	getErrors()	12	3
A	createToken()	15	3
B	checkBadips()	14	7

<?php
/*
 * You may not change or alter any portion of this comment or credits
 * of supporting developers from this source code or any supporting source code
 * which is considered copyrighted (c) material of the original comment or credit authors.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 */

namespace Xoops\Core;

use Xmf\Random;

/**
 * XOOPS security handler
 *
 * @category  Xoops\Core
 * @package   Security
 * @author    Kazumi Ono <[email protected]>
 * @author    Jan Pedersen <[email protected]>
 * @author    John Neill <[email protected]>
 * @author    Richard Griffith <[email protected]>
 * @copyright 2014-2015 XOOPS Project (http://xoops.org)
 * @license   GNU GPL 2 or later (http://www.gnu.org/licenses/gpl-2.0.html)
 * @version   Release: 1.0
 * @link      http://xoops.org
 * @since     2.0.0
 */
class Security
{
    private $errors = array();

    /**
     * Check if there is a valid token in $_REQUEST[$name . '_REQUEST']
     *
     * @param bool         $clearIfValid whether to clear the token after validation
     * @param string|false $token        token to validate
     * @param string       $name         name of session variable
     *
     * @return bool
     */
    public function check($clearIfValid = true, $token = false, $name = 'XOOPS_TOKEN')
    {
        return $this->validateToken($token, $clearIfValid, $name);
    }

    /**
     * Create a token in the user's session
     *
     * @param int    $timeout time in seconds the token should be valid
     * @param string $name    name of session variable
     *
     * @return string token value
     */
    public function createToken($timeout = 300, $name = 'XOOPS_TOKEN')
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $this->garbageCollection($name);
        $timeout = ($timeout <= 0) ? 300 : $timeout;
        $token_id = Random::generateOneTimeToken();
        // save token data on the server
        if (!isset($_SESSION[$name . '_SESSION'])) {
            $_SESSION[$name . '_SESSION'] = array();
        }
        $token_data = array(
            'id' => $token_id, 'expire' => time() + (int)($timeout)
        );
        array_push($_SESSION[$name . '_SESSION'], $token_data);
        return $token_id;
    }

    /**
     * Check if a token is valid. If no token is specified, $_REQUEST[$name . '_REQUEST'] is checked
     *
     * @param string|false $token        token to validate
     * @param bool         $clearIfValid whether to clear the token value if valid
     * @param string       $name         session name to validate
     *
     * @return bool
     */
    public function validateToken($token = false, $clearIfValid = true, $name = 'XOOPS_TOKEN')
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $ret = false;
        $log = array();
        $token = ($token !== false)
            ? $token
            : (isset($_REQUEST[$name . '_REQUEST']) ? $_REQUEST[$name . '_REQUEST'] : '');
        if (empty($token) || empty($_SESSION[$name . '_SESSION'])) {
            $str = 'No valid token found in request/session';
            $this->setErrors($str);
            $log[] = array('Token Validation', $str);
        } else {
            $token_data =& $_SESSION[$name . '_SESSION'];
            if (is_array($token_data)) {
                foreach (array_keys($token_data) as $i) {
                    if ($token === $token_data[$i]['id']) {
                        if ($this->filterToken($token_data[$i])) {
                            if ($clearIfValid) {
                                // token should be valid once, so clear it once validated
                                unset($token_data[$i]);
                            }
                            $log[] = array('Token Validation', 'Valid token found');
                            $ret = true;
                        } else {
                            $str = 'Valid token expired';
                            $this->setErrors($str);
                            $log[] = array('Token Validation', $str);
                        }
                    }
                }
            }
            if (!$ret) {
                $log[] = array('Token Validation', 'No valid token found');
            }
            $this->garbageCollection($name);
        }
        \Xoops::getInstance()->events()->triggerEvent('core.security.validatetoken.end', array($log));
        return $ret;
    }

    /**
     * Clear all token values from user's session
     *
     * @param string $name session name
     *
     * @return void
     */
    public function clearTokens($name = 'XOOPS_TOKEN')
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $_SESSION[$name . '_SESSION'] = array();
    }

    /**
     * Check whether a token value is expired or not
     *
     * @param string $token token
     *
     * @return bool
     */
    public function filterToken($token)
    {
        return (!empty($token['expire']) && $token['expire'] >= time());
    }

    /**
     * Perform garbage collection, clearing expired tokens
     *
     * @param string $name session name
     *
     * @return void
     */
    public function garbageCollection($name = 'XOOPS_TOKEN')
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $sessionName = $name . '_SESSION';
        if (!empty($_SESSION[$sessionName]) && is_array($_SESSION[$sessionName])) {
            $_SESSION[$sessionName] = array_filter($_SESSION[$sessionName], array($this, 'filterToken'));
        }
    }

    /**
     * Check the user agent's HTTP REFERER against XOOPS_URL
     *
     * @param int $docheck 0 to not check the referer (used with XML-RPC), 1 to actively check it
     *
     * @return bool
     */
    public function checkReferer($docheck = 1)
    {
        $ref = \Xoops::getInstance()->getEnv('HTTP_REFERER');
        if ($docheck == 0) {
            return true;
        }
        if ($ref == '') {
            return false;
        }
        if (strpos($ref, \XoopsBaseConfig::get('url')) !== 0) {
            return false;
        }
        return true;
    }

    /**
     * Check if visitor's IP address is banned
     * Should be changed to return bool and let the action be up to the calling script
     *
     * @return void
     */
    public function checkBadips()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $xoops = \Xoops::getInstance();
        if ($xoops->getConfig('enable_badips') == 1
            && isset($_SERVER['REMOTE_ADDR'])
            && $_SERVER['REMOTE_ADDR'] != ''
        ) {
            foreach ($xoops->getConfig('bad_ips') as $bi) {
                if (!empty($bi) && preg_match('/' . $bi . '/', $_SERVER['REMOTE_ADDR'])) {
                    exit();

                }
            }
        }
    }

    /**
     * Get the HTML code for a Xoops\Form\Token object - provides a hidden token field
     * used in forms that do not use Xoops\Form elements
     *
     * @param string $name session token name
     *
     * @return string
     */
    public function getTokenHTML($name = 'XOOPS_TOKEN')
    {
        $token = new \Xoops\Form\Token($name);
        return $token->render();
    }

    /**
     * Add an error
     *
     * @param string $error message
     *
     * @return void
     */
    public function setErrors($error)
    {
        $this->errors[] = trim($error);
    }

    /**
     * Get generated errors
     *
     * @param bool $ashtml Format using HTML?
     *
     * @return array|string Array of array messages OR HTML string
     */
    public function getErrors($ashtml = false)
    {
        if (!$ashtml) {
            return $this->errors;
        } else {
            $ret = '';
            if (is_array($this->errors)) {
                $ret = implode('<br />', $this->errors) . '<br />';
            }
            return $ret;
        }
    }
}


1		<?php
2		/*
3		* You may not change or alter any portion of this comment or credits
4		* of supporting developers from this source code or any supporting source code
5		* which is considered copyrighted (c) material of the original comment or credit authors.
6		*
7		* This program is distributed in the hope that it will be useful,
8		* but WITHOUT ANY WARRANTY; without even the implied warranty of
9		* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
10		*/
11
12		namespace Xoops\Core;
13
14		use Xmf\Random;
15
16		/**
17		* XOOPS security handler
18		*
19		* @category Xoops\Core
20		* @package Security
21		* @author Kazumi Ono <[email protected]>
22		* @author Jan Pedersen <[email protected]>
23		* @author John Neill <[email protected]>
24		* @author Richard Griffith <[email protected]>
25		* @copyright 2014-2015 XOOPS Project (http://xoops.org)
26		* @license GNU GPL 2 or later (http://www.gnu.org/licenses/gpl-2.0.html)
27		* @version Release: 1.0
28		* @link http://xoops.org
29		* @since 2.0.0
30		*/
31		class Security
32		{
33		private $errors = array();
34
35		/**
36		* Check if there is a valid token in $_REQUEST[$name . '_REQUEST']
37		*
38		* @param bool $clearIfValid whether to clear the token after validation
39		* @param string\|false $token token to validate
40		* @param string $name name of session variable
41		*
42		* @return bool
43		*/
44	1	public function check($clearIfValid = true, $token = false, $name = 'XOOPS_TOKEN')
45		{
46	1	return $this->validateToken($token, $clearIfValid, $name);
47		}
48
49		/**
50		* Create a token in the user's session
51		*
52		* @param int $timeout time in seconds the token should be valid
53		* @param string $name name of session variable
54		*
55		* @return string token value
56		*/
57	10	public function createToken($timeout = 300, $name = 'XOOPS_TOKEN')
		0 ignored issues – show Coding Style introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report `createToken` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
58		{
59	10	$this->garbageCollection($name);
60	10	$timeout = ($timeout <= 0) ? 300 : $timeout;
61	10	$token_id = Random::generateOneTimeToken();
62		// save token data on the server
63	10	if (!isset($_SESSION[$name . '_SESSION'])) {
64	7	$_SESSION[$name . '_SESSION'] = array();
65		}
66		$token_data = array(
67	10	'id' => $token_id, 'expire' => time() + (int)($timeout)
68		);
69	10	array_push($_SESSION[$name . '_SESSION'], $token_data);
70	10	return $token_id;
71		}
72
73		/**
74		* Check if a token is valid. If no token is specified, $_REQUEST[$name . '_REQUEST'] is checked
75		*
76		* @param string\|false $token token to validate
77		* @param bool $clearIfValid whether to clear the token value if valid
78		* @param string $name session name to validate
79		*
80		* @return bool
81		*/
82	2	public function validateToken($token = false, $clearIfValid = true, $name = 'XOOPS_TOKEN')
		0 ignored issues – show Coding Style introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report `validateToken` uses the super-global variable `$_REQUEST` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history... Coding Style introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report `validateToken` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
83		{
84	2	$ret = false;
85	2	$log = array();
86	2	$token = ($token !== false)
87	2	? $token
88	2	: (isset($_REQUEST[$name . '_REQUEST']) ? $_REQUEST[$name . '_REQUEST'] : '');
89	2	if (empty($token) \|\| empty($_SESSION[$name . '_SESSION'])) {
90	2	$str = 'No valid token found in request/session';
91	2	$this->setErrors($str);
92	2	$log[] = array('Token Validation', $str);
93		} else {
94	2	$token_data =& $_SESSION[$name . '_SESSION'];
95	2	if (is_array($token_data)) {
96	2	foreach (array_keys($token_data) as $i) {
97	2	if ($token === $token_data[$i]['id']) {
98	2	if ($this->filterToken($token_data[$i])) {
99	2	if ($clearIfValid) {
100		// token should be valid once, so clear it once validated
101	2	unset($token_data[$i]);
102		}
103	2	$log[] = array('Token Validation', 'Valid token found');
104	2	$ret = true;
105		} else {
106	2	$str = 'Valid token expired';
107	2	$this->setErrors($str);
108	2	$log[] = array('Token Validation', $str);
109		}
110		}
111		}
112		}
113	2	if (!$ret) {
114	2	$log[] = array('Token Validation', 'No valid token found');
115		}
116	2	$this->garbageCollection($name);
117		}
118	2	\Xoops::getInstance()->events()->triggerEvent('core.security.validatetoken.end', array($log));
119	2	return $ret;
120		}
121
122		/**
123		* Clear all token values from user's session
124		*
125		* @param string $name session name
126		*
127		* @return void
128		*/
129	1	public function clearTokens($name = 'XOOPS_TOKEN')
		0 ignored issues – show Coding Style introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report `clearTokens` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
130		{
131	1	$_SESSION[$name . '_SESSION'] = array();
132	1	}
133
134		/**
135		* Check whether a token value is expired or not
136		*
137		* @param string $token token
138		*
139		* @return bool
140		*/
141	8	public function filterToken($token)
142		{
143	8	return (!empty($token['expire']) && $token['expire'] >= time());
144		}
145
146		/**
147		* Perform garbage collection, clearing expired tokens
148		*
149		* @param string $name session name
150		*
151		* @return void
152		*/
153	10	public function garbageCollection($name = 'XOOPS_TOKEN')
		0 ignored issues – show Coding Style introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report `garbageCollection` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
154		{
155	10	$sessionName = $name . '_SESSION';
156	10	if (!empty($_SESSION[$sessionName]) && is_array($_SESSION[$sessionName])) {
157	8	$_SESSION[$sessionName] = array_filter($_SESSION[$sessionName], array($this, 'filterToken'));
158		}
159	10	}
160
161		/**
162		* Check the user agent's HTTP REFERER against XOOPS_URL
163		*
164		* @param int $docheck 0 to not check the referer (used with XML-RPC), 1 to actively check it
165		*
166		* @return bool
167		*/
168	1	public function checkReferer($docheck = 1)
169		{
170	1	$ref = \Xoops::getInstance()->getEnv('HTTP_REFERER');
171	1	if ($docheck == 0) {
172	1	return true;
173		}
174	1	if ($ref == '') {
175		return false;
176		}
177	1	if (strpos($ref, \XoopsBaseConfig::get('url')) !== 0) {
178	1	return false;
179		}
180	1	return true;
181		}
182
183		/**
184		* Check if visitor's IP address is banned
185		* Should be changed to return bool and let the action be up to the calling script
186		*
187		* @return void
188		*/
189	1	public function checkBadips()
		0 ignored issues – show Coding Style introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report `checkBadips` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
190		{
191	1	$xoops = \Xoops::getInstance();
192	1	if ($xoops->getConfig('enable_badips') == 1
193	1	&& isset($_SERVER['REMOTE_ADDR'])
194	1	&& $_SERVER['REMOTE_ADDR'] != ''
195		) {
196	1	foreach ($xoops->getConfig('bad_ips') as $bi) {
197	1	if (!empty($bi) && preg_match('/' . $bi . '/', $_SERVER['REMOTE_ADDR'])) {
198		exit();
		0 ignored issues – show Coding Style Compatibility introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report The method `checkBadips()` contains an exit expression. An exit expression should only be used in rare cases. For example, if you write a short command line script. In most cases however, using an `exit` expression makes the code untestable and often causes incompatibilities with other libraries. Thus, unless you are absolutely sure it is required here, we recommend to refactor your code to avoid its usage. Loading history...
199		}
200		}
201		}
202	1	}
203
204		/**
205		* Get the HTML code for a Xoops\Form\Token object - provides a hidden token field
206		* used in forms that do not use Xoops\Form elements
207		*
208		* @param string $name session token name
209		*
210		* @return string
211		*/
212	2	public function getTokenHTML($name = 'XOOPS_TOKEN')
213		{
214	2	$token = new \Xoops\Form\Token($name);
215	2	return $token->render();
216		}
217
218		/**
219		* Add an error
220		*
221		* @param string $error message
222		*
223		* @return void
224		*/
225	3	public function setErrors($error)
226		{
227	3	$this->errors[] = trim($error);
228	3	}
229
230		/**
231		* Get generated errors
232		*
233		* @param bool $ashtml Format using HTML?
234		*
235		* @return array\|string Array of array messages OR HTML string
236		*/
237	3	public function getErrors($ashtml = false)
238		{
239	3	if (!$ashtml) {
240	3	return $this->errors;
241		} else {
242	1	$ret = '';
243	1	if (is_array($this->errors)) {
244	1	$ret = implode('<br />', $this->errors) . '<br />';
245		}
246	1	return $ret;
247		}
248		}
249		}
250

XOOPS / XoopsCore

Push — master ( c622f7...33a06a )

Security B

Complexity

Size/Duplication

Coupling/Cohesion

Test Coverage

Importance

11 Methods

Duplication Side-by-Side

Filter issues like