Issues in acp_module.php (develop-stable) - Issues in develop-stable - paul999/ajax-shoutbox-ext - Measure and Improve Code Quality continuously with Scrutinizer

Issues (36)

Security Analysis not enabled

Cross-Site Scripting

Cross-Site Scripting enables an attacker to inject code into the response of a web-request that is viewed by other users. It can for example be used to bypass access controls, or even to take over other users' accounts.

File Exposure

File Exposure allows an attacker to gain access to local files that he should not be able to access. These files can for example include database credentials, or other configuration files.

File Manipulation

File Manipulation enables an attacker to write custom data to files. This potentially leads to injection of arbitrary code on the server.

Object Injection

Object Injection enables an attacker to inject an object into PHP code, and can lead to arbitrary code execution, file exposure, or file manipulation attacks.

Code Injection

Code Injection enables an attacker to execute arbitrary code on the server.

Response Splitting

Response Splitting can be used to send arbitrary responses.

File Inclusion

File Inclusion enables an attacker to inject custom files into PHP's file loading mechanism, either explicitly passed to include, or for example via PHP's auto-loading mechanism.

Command Injection

Command Injection enables an attacker to inject a shell command that is execute with the privileges of the web-server. This can be used to expose sensitive data, or gain access of your server.

SQL Injection

SQL Injection enables an attacker to execute arbitrary SQL code on your database server gaining access to user data, or manipulating user data.

XPath Injection

XPath Injection enables an attacker to modify the parts of XML document that are read. If that XML document is for example used for authentication, this can lead to further vulnerabilities similar to SQL Injection.

LDAP Injection

LDAP Injection enables an attacker to inject LDAP statements potentially granting permission to run unauthorized queries, or modify content inside the LDAP tree.

Header Injection

Other Vulnerability

This category comprises other attack vectors such as manipulating the PHP runtime, loading custom extensions, freezing the runtime, or similar.

Regex Injection

Regex Injection enables an attacker to execute arbitrary code in your PHP process.

XML Injection

XML Injection enables an attacker to read files on your local filesystem including configuration files, or can be abused to freeze your web-server process.

Variable Injection

Variable Injection enables an attacker to overwrite program variables with custom data, and can lead to further vulnerabilities.

Unfortunately, the security analysis is currently not available for your project. If you are a non-commercial open-source project, please contact support to gain access.

acp/acp_module.php (6 issues)

Labels

Severity

Minor 6

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
/**
 *
 * Ajax Shoutbox extension for the phpBB Forum Software package.
 *
 * @copyright (c) 2014 Paul Sohier <http://www.ajax-shoutbox.com>
 * @license GNU General Public License, version 2 (GPL-2.0)
 *
 */

namespace paul999\ajaxshoutbox\acp;

class acp_module {
	/** @var  string */
	public $u_action;

	/** @var array  */
	public $new_config = array();

	/** @var   */
	public $page_title;

	/** @var   */
	public $tpl_name;

	function main($id, $mode)
// 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);
    }
}
	{
		global $user, $template, $request;
function myFunction($a, $b) {
    // Do something
}
		global $config, $phpbb_dispatcher, $phpbb_log;
function myFunction($a, $b) {
    // Do something
}

		$user->add_lang_ext("paul999/ajaxshoutbox", "acp_ajax_shoutbox");
		$user->add_lang('acp/board');

		$submit = $request->is_set('submit');

		$form_key = 'acp_shoutbox';
		add_form_key($form_key);

		$display_vars = array(
			'title'	=> 'ACP_AJAXSHOUTBOX_SETTINGS',
			'vars'	=> array(
				'legend1'				=> 'ACP_AJAXSHOUTBOX_PRUNE',
				'ajaxshoutbox_enable_prune'			=> array('lang' => 'AJAXSHOUTBOX_ENABLE_PRUNE',			'validate' => 'bool',	'type' => 'radio:yes_no','explain' => false),
				'ajaxshoutbox_prune_days'			=> array('lang' => 'AJAXSHOUTBOX_PRUNE_DAYS',			'validate' => 'int',	'type' => 'number:1:9999','explain' => false, 'append' => ' ' . $user->lang['DAYS']),
				'legend2'				=> 'ACP_AJAXSHOUTBOX_SETTINGS',
				'ajaxshoutbox_date_format'      	=> array('lang' => 'AJAXSHOUTBOX_DEFAULT_DATE_FORMAT',	'validate' => 'string',	'type' => 'custom', 'method' => 'dateformat_select', 'explain' => true),

				'legend4'				=> 'ACP_SUBMIT_CHANGES',
			)
		);

		/**
		* Event to add and/or modify acp_board configurations
		*
		* @event paul999.ajaxshoutbox.shoutbox_config_edit_add
		* @var	array	display_vars	Array of config values to display and process
		* @var	string	mode			Mode of the config page we are displaying
		* @var	boolean	submit			Do we display the form or process the submission
		* @since 1.0.0-b2
		*/
		$vars = array('display_vars', 'mode', 'submit');
		extract($phpbb_dispatcher->trigger_event('paul999.ajaxshoutbox.shoutbox_config_edit_add', compact($vars)));

		$this->new_config = $config;
		// Copied from acp_board.php
		$cfg_array = (isset($_REQUEST['config'])) ? utf8_normalize_nfc($request->variable('config', array('' => ''), true)) : $this->new_config;
		$error = array();

		// We validate the complete config if wished
		validate_config_vars($display_vars['vars'], $cfg_array, $error);

		if ($submit && !check_form_key($form_key))
		{
			$error[] = $user->lang['FORM_INVALID'];
		}
		// Do not write values if there is an error
		if (sizeof($error))
		{
			$submit = false;
		}

		// We go through the display_vars to make sure no one is trying to set variables he/she is not allowed to...
		foreach ($display_vars['vars'] as $config_name => $null)
		{
			if (!isset($cfg_array[$config_name]) || strpos($config_name, 'legend') !== false || $config_name == 'ajaxshoutbox_validation_id' || $config_name == 'ajaxshoutbox_push_disabled')
			{
				continue;
			}

			$this->new_config[$config_name] = $config_value = $cfg_array[$config_name];

			if ($submit)
			{
				$config->set($config_name, $config_value);
			}
		}

		if ($submit)
		{
			$phpbb_log->add('admin', $user->data['user_id'], $user->ip, 'LOG_AJAX_SHOUTBOX_CONFIG_' . strtoupper($mode));

			$message = $user->lang('CONFIG_UPDATED');
			$message_type = E_USER_NOTICE;

			trigger_error($message . adm_back_link($this->u_action), $message_type);
		}

		$this->tpl_name = 'acp_board';
		$this->page_title = $display_vars['title'];

		$template->assign_vars(array(
			'L_TITLE'			=> $user->lang[$display_vars['title']],
			'L_TITLE_EXPLAIN'	=> $user->lang[$display_vars['title'] . '_EXPLAIN'],

			'S_ERROR'			=> (sizeof($error)) ? true : false,
			'ERROR_MSG'			=> implode('<br />', $error),

			'U_ACTION'			=> $this->u_action,
		));

		// Output relevant page
		foreach ($display_vars['vars'] as $config_key => $vars)
		{
			if (!is_array($vars) && strpos($config_key, 'legend') === false)
			{
				continue;
			}

			if (strpos($config_key, 'legend') !== false)
			{
				$template->assign_block_vars('options', array(
					'S_LEGEND'		=> true,
					'LEGEND'		=> (isset($user->lang[$vars])) ? $user->lang[$vars] : $vars)
				);

				continue;
			}

			$type = explode(':', $vars['type']);

			$l_explain = '';
			if ($vars['explain'] && isset($user->lang[$vars['lang'] . '_EXPLAIN']))
			{
				$l_explain =  $user->lang[$vars['lang'] . '_EXPLAIN'];
			}

			$content = build_cfg_template($type, $config_key, $this->new_config, $config_key, $vars);

			if (empty($content))
			{
				continue;
			}

			$template->assign_block_vars('options', array(
				'KEY'			=> $config_key,
				'TITLE'			=> (isset($user->lang[$vars['lang']])) ? $user->lang[$vars['lang']] : $vars['lang'],
				'S_EXPLAIN'		=> $vars['explain'],
				'TITLE_EXPLAIN'	=> $l_explain,
				'CONTENT'		=> $content,

			));

			unset($display_vars['vars'][$config_key]);
		}
	}

	/**
	 * Set the date format.
	 * @param $value
	 * @param $key
	 *
	 * @return string
	 */
	function dateformat_select($value, $key)

	{
		global $user, $config;
function myFunction($a, $b) {
    // Do something
}
		// Let the format_date function operate with the acp values
		$old_tz = $user->timezone;
		try
		{
			$user->timezone = new \DateTimeZone($config['board_timezone']);
		}
		catch (\Exception $e)
		{
			// If the board timezone is invalid, we just use the users timezone.
		}
		$dateformat_options = '';
		foreach ($user->lang['dateformats'] as $format => $null)
		{
			if (strpos($format, '|') === 0) // Skip relative formats!
			{
				continue;
			}

			$dateformat_options .= '<option value="' . $format . '"' . (($format == $value) ? ' selected="selected"' : '') . '>';
			$dateformat_options .= $user->format_date(time(), $format, false) . ((strpos($format, '|') !== false) ? $user->lang['VARIANT_DATE_SEPARATOR'] . $user->format_date(time(), $format, true) : '');
			$dateformat_options .= '</option>';
		}
		$dateformat_options .= '<option value="custom"';
		if (!isset($user->lang['dateformats'][$value]))
		{
			$dateformat_options .= ' selected="selected"';
		}
		$dateformat_options .= '>' . $user->lang['CUSTOM_DATEFORMAT'] . '</option>';
		// Reset users date options
		$user->timezone = $old_tz;
		return "<select name=\"dateoptions\" id=\"dateoptions\" onchange=\"if (this.value == 'custom') { document.getElementById('" . addslashes($key) . "').value = '" . addslashes($value) . "'; } else { document.getElementById('" . addslashes($key) . "').value = this.value; }\">$dateformat_options</select>
		<input type=\"text\" name=\"config[$key]\" id=\"$key\" value=\"$value\" maxlength=\"30\" />";
	}
}


Issues (36)

Security Analysis not enabled

acp/acp_module.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. Pass all data via parameters

2. Create a class that maintains your state

1			<?php
2			/**
3			*
4			* Ajax Shoutbox extension for the phpBB Forum Software package.
5			*
6			* @copyright (c) 2014 Paul Sohier <http://www.ajax-shoutbox.com>
7			* @license GNU General Public License, version 2 (GPL-2.0)
8			*
9			*/
10
11			namespace paul999\ajaxshoutbox\acp;
12
13			class acp_module {
14			/** @var string */
15			public $u_action;
16
17			/** @var array */
18			public $new_config = array();
19
20			/** @var */
21			public $page_title;
22
23			/** @var */
24			public $tpl_name;
25
26			function main($id, $mode)
			0 ignored issues – show Best Practice introduced 2015-12-09 13:37 UTC by Report Bug Copy Issue Report Show Similar Issues like this It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history... Coding Style introduced 2015-12-19 20:34 UTC by Report Bug Copy Issue Report Show Similar Issues like this `main` 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...
27			{
28			global $user, $template, $request;
			0 ignored issues – show Compatibility Best Practice introduced 2015-12-19 20:34 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...
29			global $config, $phpbb_dispatcher, $phpbb_log;
			0 ignored issues – show Compatibility Best Practice introduced 2015-12-19 20:34 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...
30
31			$user->add_lang_ext("paul999/ajaxshoutbox", "acp_ajax_shoutbox");
32			$user->add_lang('acp/board');
33
34			$submit = $request->is_set('submit');
35
36			$form_key = 'acp_shoutbox';
37			add_form_key($form_key);
38
39			$display_vars = array(
40			'title' => 'ACP_AJAXSHOUTBOX_SETTINGS',
41			'vars' => array(
42			'legend1' => 'ACP_AJAXSHOUTBOX_PRUNE',
43			'ajaxshoutbox_enable_prune' => array('lang' => 'AJAXSHOUTBOX_ENABLE_PRUNE', 'validate' => 'bool', 'type' => 'radio:yes_no','explain' => false),
44			'ajaxshoutbox_prune_days' => array('lang' => 'AJAXSHOUTBOX_PRUNE_DAYS', 'validate' => 'int', 'type' => 'number:1:9999','explain' => false, 'append' => ' ' . $user->lang['DAYS']),
45			'legend2' => 'ACP_AJAXSHOUTBOX_SETTINGS',
46			'ajaxshoutbox_date_format' => array('lang' => 'AJAXSHOUTBOX_DEFAULT_DATE_FORMAT', 'validate' => 'string', 'type' => 'custom', 'method' => 'dateformat_select', 'explain' => true),
47
48			'legend4' => 'ACP_SUBMIT_CHANGES',
49			)
50			);
51
52			/**
53			* Event to add and/or modify acp_board configurations
54			*
55			* @event paul999.ajaxshoutbox.shoutbox_config_edit_add
56			* @var array display_vars Array of config values to display and process
57			* @var string mode Mode of the config page we are displaying
58			* @var boolean submit Do we display the form or process the submission
59			* @since 1.0.0-b2
60			*/
61			$vars = array('display_vars', 'mode', 'submit');
62			extract($phpbb_dispatcher->trigger_event('paul999.ajaxshoutbox.shoutbox_config_edit_add', compact($vars)));
63
64			$this->new_config = $config;
65			// Copied from acp_board.php
66			$cfg_array = (isset($_REQUEST['config'])) ? utf8_normalize_nfc($request->variable('config', array('' => ''), true)) : $this->new_config;
67			$error = array();
68
69			// We validate the complete config if wished
70			validate_config_vars($display_vars['vars'], $cfg_array, $error);
71
72			if ($submit && !check_form_key($form_key))
73			{
74			$error[] = $user->lang['FORM_INVALID'];
75			}
76			// Do not write values if there is an error
77			if (sizeof($error))
78			{
79			$submit = false;
80			}
81
82			// We go through the display_vars to make sure no one is trying to set variables he/she is not allowed to...
83			foreach ($display_vars['vars'] as $config_name => $null)
84			{
85			if (!isset($cfg_array[$config_name]) \|\| strpos($config_name, 'legend') !== false \|\| $config_name == 'ajaxshoutbox_validation_id' \|\| $config_name == 'ajaxshoutbox_push_disabled')
86			{
87			continue;
88			}
89
90			$this->new_config[$config_name] = $config_value = $cfg_array[$config_name];
91
92			if ($submit)
93			{
94			$config->set($config_name, $config_value);
95			}
96			}
97
98			if ($submit)
99			{
100			$phpbb_log->add('admin', $user->data['user_id'], $user->ip, 'LOG_AJAX_SHOUTBOX_CONFIG_' . strtoupper($mode));
101
102			$message = $user->lang('CONFIG_UPDATED');
103			$message_type = E_USER_NOTICE;
104
105			trigger_error($message . adm_back_link($this->u_action), $message_type);
106			}
107
108			$this->tpl_name = 'acp_board';
109			$this->page_title = $display_vars['title'];
110
111			$template->assign_vars(array(
112			'L_TITLE' => $user->lang[$display_vars['title']],
113			'L_TITLE_EXPLAIN' => $user->lang[$display_vars['title'] . '_EXPLAIN'],
114
115			'S_ERROR' => (sizeof($error)) ? true : false,
116			'ERROR_MSG' => implode('<br />', $error),
117
118			'U_ACTION' => $this->u_action,
119			));
120
121			// Output relevant page
122			foreach ($display_vars['vars'] as $config_key => $vars)
123			{
124			if (!is_array($vars) && strpos($config_key, 'legend') === false)
125			{
126			continue;
127			}
128
129			if (strpos($config_key, 'legend') !== false)
130			{
131			$template->assign_block_vars('options', array(
132			'S_LEGEND' => true,
133			'LEGEND' => (isset($user->lang[$vars])) ? $user->lang[$vars] : $vars)
134			);
135
136			continue;
137			}
138
139			$type = explode(':', $vars['type']);
140
141			$l_explain = '';
142			if ($vars['explain'] && isset($user->lang[$vars['lang'] . '_EXPLAIN']))
143			{
144			$l_explain = $user->lang[$vars['lang'] . '_EXPLAIN'];
145			}
146
147			$content = build_cfg_template($type, $config_key, $this->new_config, $config_key, $vars);
148
149			if (empty($content))
150			{
151			continue;
152			}
153
154			$template->assign_block_vars('options', array(
155			'KEY' => $config_key,
156			'TITLE' => (isset($user->lang[$vars['lang']])) ? $user->lang[$vars['lang']] : $vars['lang'],
157			'S_EXPLAIN' => $vars['explain'],
158			'TITLE_EXPLAIN' => $l_explain,
159			'CONTENT' => $content,
160
161			));
162
163			unset($display_vars['vars'][$config_key]);
164			}
165			}
166
167			/**
168			* Set the date format.
169			* @param $value
170			* @param $key
171			*
172			* @return string
173			*/
174			function dateformat_select($value, $key)
			0 ignored issues – show Best Practice introduced 2015-12-09 13:37 UTC by Report Bug Copy Issue Report Show Similar Issues like this It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history...
175			{
176			global $user, $config;
			0 ignored issues – show Compatibility Best Practice introduced 2015-12-19 20:34 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...
177			// Let the format_date function operate with the acp values
178			$old_tz = $user->timezone;
179			try
180			{
181			$user->timezone = new \DateTimeZone($config['board_timezone']);
182			}
183			catch (\Exception $e)
184			{
185			// If the board timezone is invalid, we just use the users timezone.
186			}
187			$dateformat_options = '';
188			foreach ($user->lang['dateformats'] as $format => $null)
189			{
190			if (strpos($format, '\|') === 0) // Skip relative formats!
191			{
192			continue;
193			}
194
195			$dateformat_options .= '<option value="' . $format . '"' . (($format == $value) ? ' selected="selected"' : '') . '>';
196			$dateformat_options .= $user->format_date(time(), $format, false) . ((strpos($format, '\|') !== false) ? $user->lang['VARIANT_DATE_SEPARATOR'] . $user->format_date(time(), $format, true) : '');
197			$dateformat_options .= '</option>';
198			}
199			$dateformat_options .= '<option value="custom"';
200			if (!isset($user->lang['dateformats'][$value]))
201			{
202			$dateformat_options .= ' selected="selected"';
203			}
204			$dateformat_options .= '>' . $user->lang['CUSTOM_DATEFORMAT'] . '</option>';
205			// Reset users date options
206			$user->timezone = $old_tz;
207			return "<select name=\"dateoptions\" id=\"dateoptions\" onchange=\"if (this.value == 'custom') { document.getElementById('" . addslashes($key) . "').value = '" . addslashes($value) . "'; } else { document.getElementById('" . addslashes($key) . "').value = this.value; }\">$dateformat_options</select>
208			<input type=\"text\" name=\"config[$key]\" id=\"$key\" value=\"$value\" maxlength=\"30\" />";
209			}
210			}
211

paul999 / ajax-shoutbox-ext

Issues (36)

Security Analysis not enabled

acp/acp_module.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. Pass all data via parameters

2. Create a class that maintains your state

Duplication Side-by-Side

Filter issues like