Security hardening: unblock confirmation, escaping, settings validation

README.md

@@ -13,6 +13,27 @@ If you are interested in the source code, or want to contribute, please check [t
 
 For any further questions, don't hesitate to contact me under bfstop@bfroehler.info
 
+## Security changes in this release
+
+- The link in the "you were blocked" email now only asks for confirmation when
+  opened; the unblock itself is a POST request, and only works from the IP
+  address which was blocked (so mail scanners and link previews can't use it).
+- The log view and the IP information view now escape everything they show.
+- The unblock page answers with an error status if the link can't be used (400,
+  403, 404 or 500; a GET request just asks for confirmation and stays a 200),
+  unless the plugin's "Use HTTP Error" setting is off. Because the link contains
+  a secret, the page is not cached, sends no Referer and asks search engines
+  not to index it.
+- New setting "Usernames Not Matching an Account" (default: hash), see the
+  plugin's CHANGELOG.
+- New setting "IPv6 Tracking Granularity" (default: /64).
+- The "User Block Message" setting has a new value: the email with the unblock
+  link is by default only sent if the user has logged in from the blocked IP
+  address before, so it can't be used to flood somebody's inbox.
+- Sending the test email needs the permission to change the settings, and the
+  .htaccess and GeoIP database path settings are validated.
+- Subnets with a prefix length like `1e1` or `0x8` are no longer accepted.
+
 ## 2.0.0: Joomla 5/6 migration
 
 Version 2.0.0 migrates the component to PSR-4 namespaced classes

SECURITY.md

@@ -0,0 +1,37 @@
+# Security Policy
+
+## Supported versions
+
+Security fixes are made for the latest release of the BFStop plugin
+(`plg_system_bfstop`) and the BFStop component (`com_bfstop`). The two are
+released together and need to be on the same major and minor version, so
+please update both.
+
+## Reporting a vulnerability
+
+Please report suspected vulnerabilities by email to **security@bfstop.de**
+rather than in a public issue or pull request, so that a fix can be
+available before details are public.
+
+It helps to include:
+
+- the affected version(s) of plugin and component, and the Joomla! and PHP
+  versions,
+- the relevant settings (blocking mode, proxy settings, ...),
+- steps to reproduce, or a proof of concept, and what an attacker gains.
+
+## What is and isn't in scope
+
+BFStop sits in front of the login of a Joomla! site and decides which
+requests to reject, so these are the kind of reports which are most useful:
+
+- ways around a block or the failed-login counting (for example by faking
+  the client address, or by reusing an unblock link),
+- ways to get someone else blocked, or to make the plugin tie up the server,
+- ways to read or change data (settings, blocks, logged usernames) without
+  the necessary permissions, and cross-site scripting in the backend views.
+
+Settings are changed by users with the `core.admin` permission for
+`com_bfstop`. They can, among other things, choose the directory in which the
+plugin writes its `.htaccess` file, so grant that permission only to people
+you trust with the web server's configuration for your site.

admin/forms/settings.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="utf-8"?>
-<form>
+<form addrulepath="administrator/components/com_bfstop/rules">
 	<fields name="params">
 		<fieldset name="basic">
 			<field name="blockMode" type="list" default="full"
@@ -51,7 +51,8 @@
 				label="PLG_SYSTEM_BFSTOP_NOTIFY_BLOCKED_USER_LABEL"
 				description="PLG_SYSTEM_BFSTOP_NOTIFY_BLOCKED_USER_DESC">
 				<option value="0">PLG_SYSTEM_BFSTOP_DISABLED</option>
-				<option value="1">PLG_SYSTEM_BFSTOP_ENABLED</option>
+				<option value="1">PLG_SYSTEM_BFSTOP_NOTIFY_BLOCKED_USER_KNOWN</option>
+				<option value="2">PLG_SYSTEM_BFSTOP_NOTIFY_BLOCKED_USER_ANY</option>
 			</field>
 			<field name="notifyBlockedNumber" type="list" default="5"
 				label="PLG_SYSTEM_BFSTOP_NOTIFY_BLOCKED_NUMBER_LABEL"
@@ -169,6 +170,20 @@
 				<option value="64">PLG_SYSTEM_BFSTOP_INFO</option>
 				<option value="128">PLG_SYSTEM_BFSTOP_DEBUG</option>
 			</field>
+			<field name="unknownUsernameMode" type="list" default="hash"
+				label="COM_BFSTOP_SETTINGS_UNKNOWN_USERNAME_MODE_LABEL"
+				description="COM_BFSTOP_SETTINGS_UNKNOWN_USERNAME_MODE_DESC">
+				<option value="hash">COM_BFSTOP_SETTINGS_UNKNOWN_USERNAME_MODE_HASH</option>
+				<option value="plain">COM_BFSTOP_SETTINGS_UNKNOWN_USERNAME_MODE_PLAIN</option>
+			</field>
+			<field name="ipv6PrefixLength" type="list" default="64"
+				label="COM_BFSTOP_SETTINGS_IPV6_PREFIX_LABEL"
+				description="COM_BFSTOP_SETTINGS_IPV6_PREFIX_DESC">
+				<option value="64">COM_BFSTOP_SETTINGS_IPV6_PREFIX_64</option>
+				<option value="56">/56</option>
+				<option value="48">/48</option>
+				<option value="128">COM_BFSTOP_SETTINGS_IPV6_PREFIX_128</option>
+			</field>
 			<field name="useHttpError" type="list" default="1"
 				label="PLG_SYSTEM_BFSTOP_USE_HTTP_ERROR_LABEL"
 				description="PLG_SYSTEM_BFSTOP_USE_HTTP_ERROR_DESC">
@@ -180,6 +195,7 @@
 				label="PLG_SYSTEM_BFSTOP_DELETE_OLD_ENTRIES_LABEL"
 				description="PLG_SYSTEM_BFSTOP_DELETE_OLD_ENTRIES_DESC" />
 			<field name="htaccessPath" type="text" default="" showon="blockMode:htaccess"
+				validate="htaccesspath"
 				label="PLG_SYSTEM_BFSTOP_HTACCESS_PATH_LABEL"
 				description="PLG_SYSTEM_BFSTOP_HTACCESS_PATH_DESC" />
 		</fieldset>
@@ -229,6 +245,7 @@
 		</fieldset>
 		<fieldset name="geoip" label="COM_BFSTOP_SETTINGS_GEOIP_FIELDSET_LABEL">
 			<field name="geoDbPath" type="text" size="60" default=""
+				validate="geodbpath"
 				label="COM_BFSTOP_SETTINGS_GEOIP_DB_PATH_LABEL"
 				description="COM_BFSTOP_SETTINGS_GEOIP_DB_PATH_DESC" />
 		</fieldset>

admin/language/de-DE/de-DE.com_bfstop.ini

@@ -113,3 +113,13 @@ COM_BFSTOP_USERNAMESTATS_PURGE_INVALID_AGE="Bitte eine Anzahl an Tagen von minde
 COM_BFSTOP_USERNAMESTATS_PURGE_FAILED="Löschen der Statistik veralteter Benutzernamen fehlgeschlagen: %s"
 COM_BFSTOP_FAILEDLOGIN_FILTERED_BY_USERNAME="Es werden nur fehlgeschlagene Logins für den Benutzernamen '%s' angezeigt."
 COM_BFSTOP_FAILEDLOGIN_FILTER_CLEAR="Alle anzeigen"
+COM_BFSTOP_SETTINGS_UNKNOWN_USERNAME_MODE_LABEL="Benutzernamen ohne passendes Konto"
+COM_BFSTOP_SETTINGS_UNKNOWN_USERNAME_MODE_DESC="Benutzer tippen oft ihr Passwort in das Benutzernamenfeld. Mit 'Hash' wird der Benutzername eines fehlgeschlagenen Logins nur dann so wie eingegeben gespeichert (angezeigt, protokolliert und per E-Mail gesendet), wenn er zu einem bestehenden Konto gehört oder in der Liste der häufigen Benutzernamen steht; alles andere wird durch einen unlesbaren Wert ersetzt, der für dieselbe Eingabe immer gleich ist, sodass Versuche damit zusammen gezählt werden. 'Wie eingegeben speichern' lässt jeden eingegebenen Benutzernamen lesbar, speichert dann aber auch Passwörter, die ins Benutzernamenfeld getippt wurden."
+COM_BFSTOP_SETTINGS_UNKNOWN_USERNAME_MODE_HASH="Hash (empfohlen)"
+COM_BFSTOP_SETTINGS_UNKNOWN_USERNAME_MODE_PLAIN="Wie eingegeben speichern"
+COM_BFSTOP_SETTINGS_HTACCESS_PATH_INVALID="Der .htaccess-Pfad muss ein existierendes Verzeichnis im Dateisystem des Servers sein."
+COM_BFSTOP_SETTINGS_GEOIP_DB_PATH_INVALID="Der Pfad der GeoIP-Datenbank muss der Name einer MaxMind-Datenbankdatei (Endung .mmdb) im Dateisystem des Servers sein."
+COM_BFSTOP_SETTINGS_IPV6_PREFIX_LABEL="Genauigkeit bei IPv6"
+COM_BFSTOP_SETTINGS_IPV6_PREFIX_DESC="Ein Client mit IPv6-Adresse kontrolliert meist ein ganzes Netz und kann nach Belieben zu einer anderen Adresse darin wechseln. Fehlgeschlagene Logins und Sperren von IPv6-Clients werden deshalb pro Netz dieser Größe statt pro Adresse erfasst. /64 ist die Größe eines üblichen Heim- oder Mobilfunkanschlusses. Wähle eine einzelne Adresse nur, wenn du weißt, dass Clients deiner Seite sich ihr /64 mit anderen teilen, die nicht mitgesperrt werden sollen."
+COM_BFSTOP_SETTINGS_IPV6_PREFIX_64="/64 (empfohlen)"
+COM_BFSTOP_SETTINGS_IPV6_PREFIX_128="Einzelne Adresse (/128)"

admin/language/en-GB/en-GB.com_bfstop.ini

@@ -165,3 +165,13 @@ COM_BFSTOP_USERNAMESTATS_PURGE_INVALID_AGE="Please enter a number of days of at
 COM_BFSTOP_USERNAMESTATS_PURGE_FAILED="Deleting the statistics of stale usernames failed: %s"
 COM_BFSTOP_FAILEDLOGIN_FILTERED_BY_USERNAME="Only showing failed logins for username '%s'."
 COM_BFSTOP_FAILEDLOGIN_FILTER_CLEAR="Show all"
+COM_BFSTOP_SETTINGS_UNKNOWN_USERNAME_MODE_LABEL="Usernames Not Matching an Account"
+COM_BFSTOP_SETTINGS_UNKNOWN_USERNAME_MODE_DESC="Users often type their password into the username field. With 'Hash', the username of a failed login is only stored (and shown, logged and mailed) as typed if it belongs to an existing account or is on the list of common usernames; anything else is replaced by an unreadable value, which is still the same for the same input, so attempts against it are counted together. 'Store as typed' keeps every entered username readable, but then also stores passwords typed into the username field."
+COM_BFSTOP_SETTINGS_UNKNOWN_USERNAME_MODE_HASH="Hash (recommended)"
+COM_BFSTOP_SETTINGS_UNKNOWN_USERNAME_MODE_PLAIN="Store as typed"
+COM_BFSTOP_SETTINGS_HTACCESS_PATH_INVALID="The .htaccess path has to be an existing directory of the server's file system."
+COM_BFSTOP_SETTINGS_GEOIP_DB_PATH_INVALID="The GeoIP database path has to be the name of a MaxMind database file (ending in .mmdb) in the server's file system."
+COM_BFSTOP_SETTINGS_IPV6_PREFIX_LABEL="IPv6 Tracking Granularity"
+COM_BFSTOP_SETTINGS_IPV6_PREFIX_DESC="A client with an IPv6 address usually controls a whole network and can switch to another address in it at will. Failed logins and blocks of IPv6 clients are therefore tracked per network of this size instead of per address. /64 is the size of a typical home or mobile connection. Choose a single address only if you know that clients on your site share their /64 with others whom you don't want to be blocked together."
+COM_BFSTOP_SETTINGS_IPV6_PREFIX_64="/64 (recommended)"
+COM_BFSTOP_SETTINGS_IPV6_PREFIX_128="Single address (/128)"

admin/rules/geodbpath.php

@@ -0,0 +1,33 @@
+<?php
+/*
+ * @package BFStop Component (com_bfstop) for Joomla!
+ * @author Bernhard Froehler
+ * @copyright (C) Bernhard Froehler
+ * @license GNU/GPLv3 http://www.gnu.org/licenses/gpl-3.0.html
+**/
+defined('_JEXEC') or die;
+
+use Joomla\CMS\Form\Form;
+use Joomla\CMS\Form\FormRule;
+use Joomla\CMS\Language\Text;
+use Joomla\Registry\Registry;
+
+class JFormRuleGeodbpath extends FormRule
+{
+	public function test(SimpleXMLElement $element, $value, $group = null, Registry $input = null, Form $form = null)
+	{
+		$value = (string) $value;
+		if ($value === '')
+		{
+			return true;
+		}
+		// a plain file name of a MaxMind database: no stream wrapper (phar://,
+		// ftp://, ...), no control characters
+		if (preg_match('/[\x00-\x1f]/', $value) || preg_match('#^[a-z][a-z0-9+.-]*://#i', $value) ||
+			!preg_match('/\.mmdb\z/i', $value))
+		{
+			return new UnexpectedValueException(Text::_('COM_BFSTOP_SETTINGS_GEOIP_DB_PATH_INVALID'));
+		}
+		return true;
+	}
+}

admin/rules/htaccesspath.php

@@ -0,0 +1,39 @@
+<?php
+/*
+ * @package BFStop Component (com_bfstop) for Joomla!
+ * @author Bernhard Froehler
+ * @copyright (C) Bernhard Froehler
+ * @license GNU/GPLv3 http://www.gnu.org/licenses/gpl-3.0.html
+**/
+defined('_JEXEC') or die;
+
+use Joomla\CMS\Form\Form;
+use Joomla\CMS\Form\FormRule;
+use Joomla\CMS\Language\Text;
+use Joomla\Registry\Registry;
+
+class JFormRuleHtaccesspath extends FormRule
+{
+	public function test(SimpleXMLElement $element, $value, $group = null, Registry $input = null, Form $form = null)
+	{
+		$value = (string) $value;
+		if ($value === '')
+		{
+			return true;
+		}
+		// a plain path of the file system: no stream wrapper (phar://, ftp://,
+		// ...), no control characters
+		$valid = !preg_match('/[\x00-\x1f]/', $value) && !preg_match('#^[a-z][a-z0-9+.-]*://#i', $value);
+		// and, if the .htaccess file is what blocks, an existing directory:
+		// the plugin writes the file there
+		if ($valid && $input !== null && $input->get('params.blockMode') === 'htaccess')
+		{
+			$valid = is_dir($value);
+		}
+		if (!$valid)
+		{
+			return new UnexpectedValueException(Text::_('COM_BFSTOP_SETTINGS_HTACCESS_PATH_INVALID'));
+		}
+		return true;
+	}
+}

admin/src/Controller/SettingsController.php

@@ -63,6 +63,14 @@ protected function getRedirectToItemAppend($recordId = null, $urlVar = 'id')
 	public function testNotify()
 	{
 		$this->checkToken();
+		// part of the settings, so reserved for who may change them: the
+		// mail goes to the notification addresses configured there
+		if (!$this->allowSave())
+		{
+			$this->setRedirect(Route::_('index.php?option=com_bfstop&view=settings', false),
+				Text::_('COM_BFSTOP_NOT_AUTHORISED'), 'error');
+			return;
+		}
 		$emailAddress = ParamHelper::get('emailaddress', 'params', '');
 		$userID = (int) ParamHelper::get('userID', 'params', -1);
 		$userGroup = (int) ParamHelper::get('userGroup', 'params', -1);

admin/src/Helper/IpValidateHelper.php

@@ -26,7 +26,10 @@ public static function validIPRange($address)
 			$subnet = $parts[1];
 			$ip = $parts[0];
 			$maxBits = IpRangeHelper::isIPv6($ip) ? 128 : 32;
-			if (!is_numeric($subnet) || $subnet < 0 || $subnet > $maxBits)
+			// digits only: is_numeric() would also accept "1e1", "0x8" or " 8",
+			// which don't mean what they look like in the places the range ends
+			// up (e.g. the web server's configuration)
+			if (preg_match('/^[0-9]{1,3}\z/', $subnet) !== 1 || (int) $subnet > $maxBits)
 			{
 				Factory::getApplication()->enqueueMessage(Text::sprintf('COM_BFSTOP_IP_INVALID_SUBNET', $subnet), 'warning');
 				return false;

admin/src/Helper/ParamHelper.php

@@ -20,7 +20,7 @@ public static function get($name, $column, $defaultValue)
 		$sql = "SELECT $column FROM #__extensions WHERE name = 'plg_system_bfstop'";
 		$db->setQuery($sql);
 		$rawSettings = $db->loadResult();
-		$settings = json_decode($rawSettings, true);
-		return array_key_exists($name, $settings) ? $settings[$name] : $defaultValue;
+		$settings = json_decode((string) $rawSettings, true);
+		return (is_array($settings) && array_key_exists($name, $settings)) ? $settings[$name] : $defaultValue;
 	}
 }

admin/src/Model/LogModel.php

@@ -50,7 +50,7 @@ private function getLogLines($start, $count, $ordering, $direction)
 				if (count($logItems) < 3)
 				{
 					$application = Factory::getApplication();
-					$application->enqueueMessage("Invalid entry in line number ".$lineNumber.": '".$line."' (expected 3 items - date, priority and message but that line only has ".count($logItems).")", 'error');
+					$application->enqueueMessage("Invalid entry in line number ".$lineNumber.": '".htmlspecialchars($line, ENT_QUOTES, 'UTF-8')."' (expected 3 items - date, priority and message but that line only has ".count($logItems).")", 'error');
 				}
 				else
 				{

admin/src/View/Ipinfo/HtmlView.php

@@ -38,23 +38,26 @@ public function display($tpl = null)
 		}
 		else
 		{
-			$this->ipInfo = "<pre>".Text::sprintf("COM_BFSTOP_IPINFO_DETAILS",
+			// the address comes from the request and the rest from the GeoIP
+			// database file, so none of it is trusted to be free of markup
+			$this->ipInfo = "<pre>".$this->escape(Text::sprintf("COM_BFSTOP_IPINFO_DETAILS",
 				$details->ip,
 				$details->countryCode,
 				$details->countryName,
 				$details->region,
 				$details->city,
 				$details->postalCode,
 				$details->latitude,
-				$details->longitude)."</pre>";
+				$details->longitude))."</pre>";
 		}
 		$this->addToolbar();
 		parent::display($tpl);
 	}
 
 	protected function addToolbar()
 	{
-		ToolbarHelper::title(Text::sprintf('COM_BFSTOP_HEADING_IPINFO', $this->ipAddress), 'bfstop');
+		// the toolbar title is output as HTML
+		ToolbarHelper::title(Text::sprintf('COM_BFSTOP_HEADING_IPINFO', $this->escape((string) $this->ipAddress)), 'bfstop');
 		ToolbarHelper::divider();
 		ToolbarHelper::back();
 	}

admin/tmpl/log/default_body.php

@@ -8,8 +8,8 @@
 defined('_JEXEC') or die;
 foreach ($this->items as $i => $logline): ?>
 <tr>
-	<td><?php echo $logline->date; ?></td>
-	<td><?php echo $logline->priority; ?></td>
-	<td><?php echo $logline->message; ?></td>
+	<td><?php echo $this->escape($logline->date); ?></td>
+	<td><?php echo $this->escape($logline->priority); ?></td>
+	<td><?php echo $this->escape($logline->message); ?></td>
 </tr>
 <?php endforeach;

site/language/de-DE/de-DE.com_bfstop.ini

@@ -2,3 +2,6 @@ COM_BFSTOP_UNBLOCKTOKEN_HEADING="Entsperren via Token"
 COM_BFSTOP_UNBLOCKTOKEN_FAILED="Entsperren fehlgeschlagen. Entweder war das angegebene Token ungültig, es ist bereits abgelaufen, oder es ist ein Fehler aufgetreten. Bitte kontaktiere den System-Administrator."
 COM_BFSTOP_UNBLOCKTOKEN_SUCCESS="Das Entsperren deiner IP-Adresse war erfolgreich. Wenn du meinst, deine Login-Daten nun wieder zu wissen, dann logge dich über die <a href='%s'>Login-Seite</a> ein. Du wirst allerdings erneut gesperrt werden, solltest du zu oft einen falschen Benutzernamen/Passwort eingeben. Alternativ kannst du die Funktion zum <a href='%s'>Passwort zurücksetzen</a> benutzen."
 COM_BFSTOP_UNBLOCKTOKEN_INVALID="Ungültiger Aufruf!"
+COM_BFSTOP_UNBLOCKTOKEN_CONFIRM="Bitte bestätige, dass deine IP-Adresse entsperrt werden soll. Dieser Link funktioniert nur von der IP-Adresse aus, die gesperrt wurde."
+COM_BFSTOP_UNBLOCKTOKEN_BUTTON="Meine IP-Adresse entsperren"
+COM_BFSTOP_UNBLOCKTOKEN_WRONG_IP="Dieser Link kann nur die IP-Adresse entsperren, für die er ausgestellt wurde. Bitte öffne ihn erneut auf dem Gerät und im Netzwerk, von dem aus du gesperrt wurdest."

site/language/en-GB/en-GB.com_bfstop.ini

@@ -2,3 +2,6 @@ COM_BFSTOP_UNBLOCKTOKEN_HEADING="Unblock by Token"
 COM_BFSTOP_UNBLOCKTOKEN_FAILED="Could not unblock. Either the token was invalid, it has already expired or an error has occurred. Please contact the system administrator."
 COM_BFSTOP_UNBLOCKTOKEN_SUCCESS="Successfully unblocked your IP address. If you think you now know your account data, you can try again to log in on the <a href='%s'>Login Page</a>. But you will be blocked again if you enter your username/password wrong again multiple times. You can also use the <a href='%s'>Reset password</a> functionality to have password reset instructions sent to your email address."
 COM_BFSTOP_UNBLOCKTOKEN_INVALID="Invalid call!"
+COM_BFSTOP_UNBLOCKTOKEN_CONFIRM="Please confirm that you want to unblock your IP address. This link only works from the IP address which was blocked."
+COM_BFSTOP_UNBLOCKTOKEN_BUTTON="Unblock my IP address"
+COM_BFSTOP_UNBLOCKTOKEN_WRONG_IP="This link can only unblock the IP address it was issued for. Please open it again on the device and network from which you were blocked."