Fixes #8278: Bug 1 - check GET Vars but try to report problem from POST var....

Fixes #8278: Bug 1 - check GET Vars but try to report problem from POST var. Bug 2: writing log message crashed due to not initialized QFQ-config.

extension/Documentation/Manual.rst

@@ -237,14 +237,14 @@ FormEditor
 Setup a *report* to manage all *forms*:
 
 * Create a Typo3 page.
-* Set the 'URL Alias' to `form` (default) or the individual defined value in parameter `editFormPage` (configuration_).
+* Set the 'URL Alias' to `form` (recommended) or the individual defined value in parameter `editFormPage` (configuration_).
 * Insert a content record of type *qfq*.
 * In the bodytext insert the following code::
 
     # If there is a form given by SIP: show
     form={{form:SE}}
 
-    # In case indexQfq is different from indexData, set indexQfq.
+    # In case indexQfq != indexData, set dbIndex=indexQfq.
     dbIndex = {{indexQfq:Y}}
 
     10 {

extension/Source/core/Constants.php

@@ -546,6 +546,8 @@ const SYSTEM_LDAP_1_PASSWORD = 'LDAP_1_PASSWORD'; // Credentials to access LDAP
 
 const SYSTEM_ESCAPE_TYPE_DEFAULT = 'escapeTypeDefault';
 const SYSTEM_SECURITY_VARS_HONEYPOT = 'securityVarsHoneypot'; // Fake variables
+const SYSTEM_SECURITY_VARS_HONEYPOT_NAMES = 'email,username,password'; // Fake variables
+
 const SYSTEM_SECURITY_ATTACK_DELAY = 'securityAttackDelay'; // Detected attack causes x seconds delay
 const SYSTEM_SECURITY_ATTACK_DELAY_DEFAULT = 5; // Detected attack causes x seconds delay
 const SYSTEM_SECURITY_SHOW_MESSAGE = 'securityShowMessage'; // Detected attack shows an error message

extension/Source/core/helper/Logger.php

@@ -123,7 +123,9 @@ class Logger {
                     return $filename;
                 }
 
-                throw new CodeException('SystemSitePath is not set and the given logfile should be made absolute.', ERROR_MISSING_VALUE);
+                // In case of attack detection, the config is not available - extract the installation directory from the server vars.
+                return dirname($_SERVER['SCRIPT_FILENAME']) . DIRECTORY_SEPARATOR . $filename;
+//                throw new CodeException('SystemSitePath is not set and the given logfile should be made absolute.', ERROR_MISSING_VALUE);
             }
 
             return self::$systemSitePath . DIRECTORY_SEPARATOR . $filename;

extension/Source/core/store/Config.php

@@ -199,27 +199,29 @@ class Config {
      * @throws UserFormException
      * @throws UserReportException
      */
-    private static function checkForAttack(array $config) {
+    public static function checkForAttack(array $config) {
         $attack = false;
         $key = '';
-        $reason = 'please help';
+        $reason = 'Problem: ';
 
         // Iterate over all fake vars
         $arr = explode(',', $config[SYSTEM_SECURITY_VARS_HONEYPOT]);
         foreach ($arr as $key) {
+
             $key = trim($key);
             if ($key === '') {
                 continue;
             }
-            if (!empty($_POST[$key]) || !empty($_GET[$key])) {
+
+            if (!empty($_POST[$key])) {
                 $attack = true;
-                $reason .= "Post/Get Honeypot variable '$key' detected: " . htmlentities($_POST[$key] . $_GET[$key]) . PHP_EOL;
+                $reason .= "Post/Get Honeypot variable '$key' detected: " . htmlentities($_POST[$key]) . PHP_EOL;
             }
         }
 
         // Limit length of all get vars: protect against SQL injection based on long ...%34%34%24%34...
         $maxLength = $config[SYSTEM_SECURITY_GET_MAX_LENGTH];
-        if ($maxLength > 0) {
+        if ($maxLength > 0 && $attack === false) {
             foreach ($_GET as $key => $value) {
                 // Check if the variable is something like 'my_name_100' - if the part after the last '_' is numerical, this means a valid, non standard length.
                 $arr = explode(GET_EXTRA_LENGTH_TOKEN, $key);
@@ -231,10 +233,10 @@ class Config {
                     $maxLength = $config[SYSTEM_SECURITY_GET_MAX_LENGTH]; // might change again.
                 }
 
-                if (strlen($value) > $maxLength) {
+                $len = strlen($value);
+                if ($len > $maxLength) {
                     $attack = true;
-                    $reason .= "Get variable '$key' longer than $maxLength: " . htmlentities($_POST[$key]);
-                    break;
+                    $reason .= "Get variable '$key' too long. Allowed: $maxLength, Length: $len - " . htmlentities($_GET[$key]) . PHP_EOL;
                 }
             }
         }
@@ -261,7 +263,7 @@ class Config {
         }
 
         Logger::logMessage(Logger::linePre() . 'Security: attack detected' . PHP_EOL . $reason,
-            empty($config[SYSTEM_QFQ_LOG]) ? SYSTEM_QFQ_LOG_FILE : $config[SYSTEM_QFQ_LOG]);
+            $config[SYSTEM_QFQ_LOG] ?? SYSTEM_QFQ_LOG_FILE);
 
         // In case of an attack: log out the current user.
         Session::destroy();
@@ -284,6 +286,10 @@ class Config {
 //            echo json_encode($answer);
         }
 
+        if (defined('PHPUNIT_QFQ')) {
+            throw new UserFormException('Attack detected', 1);
+        }
+
         exit;
     }
 
@@ -292,7 +298,7 @@ class Config {
      *
      * @return array
      */
-    private static function setDefaults(array $config) {
+    public static function setDefaults(array $config) {
 
         $default = [
 
@@ -338,7 +344,7 @@ class Config {
 
             F_BUTTON_ON_CHANGE_CLASS => 'btn-info alert-info',
             SYSTEM_EDIT_FORM_PAGE => 'form',
-            SYSTEM_SECURITY_VARS_HONEYPOT => 'email,username,password',
+            SYSTEM_SECURITY_VARS_HONEYPOT => SYSTEM_SECURITY_VARS_HONEYPOT_NAMES,
             SYSTEM_SECURITY_ATTACK_DELAY => SYSTEM_SECURITY_ATTACK_DELAY_DEFAULT,
             SYSTEM_SECURITY_SHOW_MESSAGE => '0',
             SYSTEM_SECURITY_GET_MAX_LENGTH => SYSTEM_SECURITY_GET_MAX_LENGTH_DEFAULT,
@@ -377,17 +383,12 @@ class Config {
 
         ];
 
-        // To let run legacy code
-//        $config[SYSTEM_DB_INDEX_DATA_DEPRECATED] = $config[SYSTEM_DB_INDEX_DATA];
-//        $config[SYSTEM_DB_INDEX_QFQ_DEPRECATED] = $config[SYSTEM_DB_INDEX_QFQ];
-
         foreach ($default as $key => $value) {
             if (!isset($config[$key]) || $config[$key] == '') {
                 $config[$key] = $value;
             }
         }
 
-//        return array_merge($default, $config);
         return $config;
     }
 

extension/Source/core/store/Session.php

@@ -123,7 +123,9 @@ class Session
             setcookie(SESSION_NAME, '', time() - 86400, '/'); // empty value and old timestamp
         }
 
-        session_destroy();
+        if (!defined('PHPUNIT_QFQ')) {
+            session_destroy();
+        }
         $_SESSION = array();
 
     }

extension/Source/core/store/Store.php

@@ -93,7 +93,7 @@ class Store {
 
         // This check is critical for some unwanted exception recursion during startup.
         if (!function_exists('normalizer_normalize')) {
-            throw new CodeException("Function normalizer_normalize() not found - Please install 'php5-intl' / 'php7.0-intl'", ERROR_MISSING_INTL);
+            throw new CodeException("Function normalizer_normalize() not found - Please install 'php-intl'", ERROR_MISSING_INTL);
         }
 
         self::$sanitizeClass = [

extension/Tests/unit/core/store/ConfigTest.php

+<?php
+/**
+ * Created by PhpStorm.
+ * User: crose
+ * Date: 1/2/16
+ * Time: 9:16 PM
+ */
+
+namespace qfq;
+
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Class StoreTest
+ * @package qfq
+ */
+class ConfigTest extends TestCase {
+
+
+    /**
+     * @expectedException \qfq\UserFormException
+     *
+     * @throws CodeException
+     * @throws UserFormException
+     * @throws UserReportException
+     */
+    public function testCheckForAttack() {
+
+        $config = Config::setDefaults(array());
+
+        $_GET['fake'] = '012345678901234567890123456789012345678901234567890123456789';
+        Config::checkForAttack($config);
+    }
+
+    /**
+     *
+     * @throws CodeException
+     * @throws UserFormException
+     * @throws UserReportException
+     */
+    public function testCheckForAttack2() {
+
+        $config = Config::setDefaults(array());
+
+        // Check for customized length (below)
+        $_GET = array();
+        # 60
+        $_GET['fake_65'] = '012345678901234567890123456789012345678901234567890123456789';
+        Config::checkForAttack($config);
+
+        $this->assertEquals(1, 1);
+    }
+
+    /**
+     * @expectedException \qfq\UserFormException
+     *
+     * @throws CodeException
+     * @throws UserFormException
+     * @throws UserReportException
+     */
+    public function testCheckForAttack3() {
+
+        $config = Config::setDefaults(array());
+
+        // Check for customized length (below)
+        $_GET = array();
+
+        // Check for customized length (above)
+        # 80
+        $_GET['fake_65'] = '01234567890123456789012345678901234567890123456789012345678901234567890123456789';
+        Config::checkForAttack($config);
+
+    }
+
+    /**
+     * @expectedException \qfq\UserFormException
+     *
+     * @throws CodeException
+     * @throws UserFormException
+     * @throws UserReportException
+     */
+    public function testCheckForAttack4() {
+
+        $config = Config::setDefaults(array());
+
+        $_POST['email'] = 'no@email.com';
+        Config::checkForAttack($config);
+    }
+
+}

extension/Tests/unit/core/store/StoreTest.php

@@ -349,7 +349,7 @@ class StoreTest extends TestCase {
             SYSTEM_DB_INDEX_DATA => '1',
             SYSTEM_DB_INDEX_QFQ => '1',
             SYSTEM_ESCAPE_TYPE_DEFAULT => 'm',
-            SYSTEM_SECURITY_VARS_HONEYPOT => 'email,username,password',
+            SYSTEM_SECURITY_VARS_HONEYPOT => SYSTEM_SECURITY_VARS_HONEYPOT_NAMES,
             SYSTEM_SECURITY_ATTACK_DELAY => 5,
             SYSTEM_SECURITY_SHOW_MESSAGE => '0',
             SYSTEM_SECURITY_GET_MAX_LENGTH => 50,