fix(install): make the database identifier validators reject bad input

assets/modules/store/installer/instprocessor.php

@@ -149,7 +149,7 @@ function propertiesNameValue($propertyString) {
             if (evo()->getDatabase()->getRecordCount($rs)) {
                 $insert = true;
                 while($row = evo()->getDatabase()->getRow($rs,'assoc')) {
-                    if (!@evo()->getDatabase()->query("UPDATE `" . $table_prefix . "site_tmplvars` SET type='$input_type', caption='$caption', description='$desc', category='$category', locked='$locked', elements='$input_options', display='$output_widget', display_params='$output_widget_params', default_text='$input_default' WHERE id='{$row['id']}';")) {
+                    if (!@evo()->getDatabase()->query("UPDATE `" . $table_prefix . "site_tmplvars` SET type='$input_type', caption='$caption', description='$desc', category='$category', locked='$locked', elements='$input_options', display='$output_widget', display_params='$output_widget_params', default_text='$input_default' WHERE id='" . (int) $row['id'] . "';")) {
                         echo "<p>" . mysql_error() . "</p>";
                         return;
                     }
@@ -175,8 +175,8 @@ function propertiesNameValue($propertyString) {
                     // remove existing tv -> template assignments
                     $ds = evo()->getDatabase()->query("SELECT id FROM `".$table_prefix."site_tmplvars` WHERE name='$name' AND description='$desc';" );
                     $row = evo()->getDatabase()->getRow($ds,'assoc');
-                    $id = $row["id"];
-                    evo()->getDatabase()->query("DELETE FROM $dbase.`" . $table_prefix . "site_tmplvar_templates` WHERE tmplvarid = '$id';");
+                    $id = (int) $row["id"];
+                    evo()->getDatabase()->query("DELETE FROM `" . $table_prefix . "site_tmplvar_templates` WHERE tmplvarid = '$id';");
 
                     // add tv -> template assignments
                     foreach ($assignments as $assignment) {
@@ -186,7 +186,7 @@ function propertiesNameValue($propertyString) {
                         $ts = evo()->getDatabase()->query("SELECT id FROM `".$table_prefix."site_templates` $where;" );
                         if ($ds && $ts) {
                             $tRow = evo()->getDatabase()->getRow($ts,'assoc');
-                            $templateId = $tRow['id'];
+                            $templateId = (int) $tRow['id'];
                             evo()->getDatabase()->query("INSERT INTO `" . $table_prefix . "site_tmplvar_templates` (tmplvarid, templateid) VALUES('$id', '$templateId');");
                         }
                     }
@@ -338,13 +338,13 @@ function propertiesNameValue($propertyString) {
                     while($row = evo()->getDatabase()->getRow($rs,'assoc')) {
                         $props = evo()->getDatabase()->escape(propUpdate($properties,$row['properties']));
                         if ($row['description'] == $desc) {
-                            if (!@evo()->getDatabase()->query("UPDATE `" . $table_prefix . "site_plugins` SET plugincode='$plugin', description='$desc', properties='$props' WHERE id='{$row['id']}';")) {
+                            if (!@evo()->getDatabase()->query("UPDATE `" . $table_prefix . "site_plugins` SET plugincode='$plugin', description='$desc', properties='$props' WHERE id='" . (int) $row['id'] . "';")) {
                                 echo "<p>" . mysql_error() . "</p>";
                                 return;
                             }
                             $insert = false;
                         } else {
-                            if (!@evo()->getDatabase()->query("UPDATE `" . $table_prefix . "site_plugins` SET disabled='1' WHERE id='{$row['id']}';")) {
+                            if (!@evo()->getDatabase()->query("UPDATE `" . $table_prefix . "site_plugins` SET disabled='1' WHERE id='" . (int) $row['id'] . "';")) {
                                 echo "<p>".mysql_error()."</p>";
                                 return;
                             }
@@ -371,12 +371,13 @@ function propertiesNameValue($propertyString) {
                     $ds = evo()->getDatabase()->query("SELECT id FROM `".$table_prefix."site_plugins` WHERE name='$name' AND description='$desc' ORDER BY id DESC LIMIT 1;" );
                     if ($ds) {
                         $row = evo()->getDatabase()->getRow($ds,'assoc');
-                        $id = $row["id"];
-                        $_events = implode("','", $events);
+                        $id = (int) $row["id"];
+                        // Event names come from the package's docblock, not from this site.
+                        $_events = implode("','", evo()->getDatabase()->escape(array_map('trim', $events)));
 
                         // add new events
                         if ($prev_id) {
-                            $prev_id = EvolutionCms()->getDatabase()->escape($prev_id);
+                            $prev_id = (int) $prev_id;
 
                             evo()->getDatabase()->query("INSERT OR IGNORE INTO `{$table_prefix}site_plugin_events` (`pluginid`, `evtid`, `priority`)
                                 SELECT {$id} as 'pluginid', `se`.`id` AS `evtid`, COALESCE(`spe`.`priority`, MAX(`spe2`.`priority`) + 1, 0) AS `priority`

core/tests/Unit/Install/InstallerLanguageSelectionTest.php

@@ -0,0 +1,83 @@
+<?php
+
+namespace Tests\Unit\Install;
+
+use PHPUnit\Framework\TestCase;
+
+final class InstallerLanguageSelectionTest extends TestCase
+{
+    /**
+     * Runs install/src/lang.php in a clean PHP process with the given request data and
+     * returns the languages it settled on.
+     */
+    private function resolve(array $get = [], array $post = [], string $acceptLanguage = ''): array
+    {
+        $langFile = dirname(__DIR__, 4) . '/install/src/lang.php';
+        $script = '<?php'
+            . ' define("MGR_DIR", "manager");'
+            . ' $_GET = ' . var_export($get, true) . ';'
+            . ' $_POST = ' . var_export($post, true) . ';'
+            . ' $_SERVER["HTTP_ACCEPT_LANGUAGE"] = ' . var_export($acceptLanguage, true) . ';'
+            . ' require ' . var_export($langFile, true) . ';'
+            . ' echo json_encode(["install" => $install_language, "manager" => $manager_language,'
+            . ' "hasStrings" => count($_lang) > 0]);';
+
+        $tmp = tempnam(sys_get_temp_dir(), 'evo-lang-');
+        file_put_contents($tmp, $script);
+        try {
+            $output = shell_exec(escapeshellarg(PHP_BINARY) . ' ' . escapeshellarg($tmp) . ' 2>&1');
+        } finally {
+            unlink($tmp);
+        }
+
+        $result = json_decode((string) $output, true);
+        self::assertIsArray($result, 'lang.php failed: ' . $output);
+
+        return $result;
+    }
+
+    public function testShippedLanguageIsSelected(): void
+    {
+        $result = $this->resolve(['language' => 'de', 'managerlanguage' => 'uk']);
+
+        self::assertSame('de', $result['install']);
+        self::assertSame('uk', $result['manager']);
+        self::assertTrue($result['hasStrings']);
+    }
+
+    public function testUnknownLanguageFallsBackInsteadOfFailingTheInclude(): void
+    {
+        $result = $this->resolve(['language' => 'xx', 'managerlanguage' => 'xx']);
+
+        self::assertSame('en', $result['install']);
+        self::assertSame('en', $result['manager']);
+        self::assertTrue($result['hasStrings']);
+    }
+
+    public function testPathLikeLanguageValuesAreRejected(): void
+    {
+        $result = $this->resolve(
+            ['language' => '../../core/config/app', 'managerlanguage' => '..'],
+            ['language' => ['de']],
+            '..'
+        );
+
+        self::assertSame('en', $result['install']);
+        self::assertSame('en', $result['manager']);
+    }
+
+    public function testPostTakesPrecedenceOverQueryAndAcceptLanguage(): void
+    {
+        $result = $this->resolve(['language' => 'fr'], ['language' => 'it'], 'de-DE,de;q=0.9');
+
+        self::assertSame('it', $result['install']);
+        self::assertSame('it', $result['manager']);
+    }
+
+    public function testAcceptLanguageIsUsedWhenNothingIsRequested(): void
+    {
+        $result = $this->resolve([], [], 'nl-NL,nl;q=0.9');
+
+        self::assertSame('nl', $result['install']);
+    }
+}

core/tests/Unit/Security/InstallerIdentifierValidationTest.php

@@ -0,0 +1,120 @@
+<?php
+
+/*
+|--------------------------------------------------------------------------
+| Installer identifier validation
+|--------------------------------------------------------------------------
+|
+| The installer splices the database name, collation and table prefix into CREATE DATABASE,
+| SELECT ... FROM {prefix}site_content and the generated connection config. Its validators
+| compared preg_match() with false, which only happens on a regex error, so a mismatch passed
+| straight through and every one of them accepted anything.
+|
+| @since 3.5.8
+*/
+
+require_once dirname(__DIR__, 4) . '/install/src/functions.php';
+
+function installerRoot(string $path): string
+{
+    return (string) file_get_contents(dirname(__DIR__, 4) . '/' . $path);
+}
+
+describe('validateDbName()', function () {
+
+    test('accepts ordinary database names', function () {
+        expect(validateDbName('evo'))->toBe('evo')
+            ->and(validateDbName('my_site-2'))->toBe('my_site-2')
+            ->and(validateDbName('1site'))->toBe('1site')
+            ->and(validateDbName('app$db'))->toBe('app$db')
+            ->and(validateDbName(' `evo` '))->toBe('evo');
+    });
+
+    test('rejects names that could leave their quotes', function (string $name) {
+        expect(fn () => validateDbName($name))->toThrow(InvalidArgumentException::class);
+    })->with([
+        'backtick' => ['evo`; DROP DATABASE mysql; -- '],
+        'double quote' => ['evo" WITH OWNER postgres'],
+        'single quote' => ["evo' OR 1=1"],
+        'space' => ['evo site'],
+        'semicolon' => ['evo;'],
+        'newline' => ["evo\nDROP"],
+        'path' => ['../evo'],
+        'empty' => [''],
+    ]);
+
+    test('rejects names of 64 characters or more', function () {
+        expect(fn () => validateDbName(str_repeat('a', 64)))->toThrow(InvalidArgumentException::class);
+    });
+});
+
+describe('validateDbCollation()', function () {
+
+    test('accepts MySQL, PostgreSQL and SQLite collations', function (string $collation) {
+        expect(validateDbCollation($collation))->toBe($collation);
+    })->with([
+        'utf8mb4_unicode_ci', 'utf8mb4_0900_ai_ci', 'en_US.UTF-8', 'en_US.utf8', 'C', 'C.UTF-8',
+        'en-US-x-icu', 'sr_RS@latin', 'utf8', '',
+    ]);
+
+    test('rejects collations carrying SQL', function (string $collation) {
+        expect(fn () => validateDbCollation($collation))->toThrow(InvalidArgumentException::class);
+    })->with([
+        'unquoted clause' => ['utf8mb4_general_ci; DROP DATABASE mysql'],
+        'quote break' => ["en_US.utf8' TEMPLATE template1 --"],
+        'comment' => ['utf8mb4_bin/**/'],
+        'leading digit' => ['1collation'],
+    ]);
+});
+
+describe('validateTablePrefix()', function () {
+
+    test('accepts the prefixes the installer generates or allows', function () {
+        expect(validateTablePrefix('evo_'))->toBe('evo_')
+            ->and(validateTablePrefix('a1b2_'))->toBe('a1b2_')
+            ->and(validateTablePrefix(''))->toBe('')
+            ->and(validateTablePrefix(null))->toBe('');
+    });
+
+    test('rejects prefixes that would change the statement or the config file', function (string $prefix) {
+        expect(fn () => validateTablePrefix($prefix))->toThrow(InvalidArgumentException::class);
+    })->with([
+        'statement' => ['x; DROP TABLE users; --'],
+        'subquery' => ['(SELECT 1)'],
+        'quote' => ["evo_'"],
+        'php' => ['evo_\'.phpinfo().\''],
+        'too long' => [str_repeat('a', 45)],
+    ]);
+});
+
+describe('call sites', function () {
+
+    test('the database test validates the collation before it reaches CREATE DATABASE', function () {
+        expect(installerRoot('install/src/controllers/connection/databasetest.php'))
+            ->toContain("\$database_collation = validateDbCollation(\$_POST['database_collation'] ?? '');")
+            ->not->toContain("\$database_collation = \$_POST['database_collation'];");
+    });
+
+    test('the database test reports a rejected value instead of dying on it', function () {
+        // The validators never threw before, so nothing here caught them.
+        expect(installerRoot('install/src/controllers/connection/databasetest.php'))
+            ->toMatch('/try \{\s+\$driver = validateDbType.*?\} catch \(InvalidArgumentException \$e\) \{\s+exit\(\$output \. \'<span id="database_fail">\'/s');
+    });
+
+    test('the installer validates the prefix it writes into the connection config', function () {
+        expect(installerRoot('install/src/controllers/install.php'))
+            ->toContain("addslashes(validateTablePrefix(\$_POST['tableprefix'] ?? ''))");
+    });
+
+    test('the legacy store processor casts ids and escapes event names', function () {
+        $source = installerRoot('assets/modules/store/installer/instprocessor.php');
+
+        expect($source)
+            ->toContain('$id = (int) $row["id"];')
+            ->toContain("\$templateId = (int) \$tRow['id'];")
+            ->toContain('$prev_id = (int) $prev_id;')
+            ->toContain("evo()->getDatabase()->escape(array_map('trim', \$events))")
+            ->not->toContain("WHERE id='{\$row['id']}'")
+            ->not->toContain('DELETE FROM $dbase.');
+    });
+});

install/src/controllers/connection/databasetest.php

@@ -1,14 +1,18 @@
 <?php
-$driver = validateDbType($_POST['database_type']);
-$host = validateDbHost($_POST['host'], $driver);
-$uid = validateDbUser($_POST['uid'], $driver);
-$pwd = validateDbPassword($_POST['pwd'], $driver);
-$tableprefix = validateTablePrefix($_POST['tableprefix']);
-$database_name = validateDbName($_POST['database_name']);
-$installMode = (int)$_POST['installMode'];
-
 $output = $_lang['status_checking_database'];
-$database_collation = $_POST['database_collation'];
+try {
+    $driver = validateDbType($_POST['database_type']);
+    $host = validateDbHost($_POST['host'], $driver);
+    $uid = validateDbUser($_POST['uid'], $driver);
+    $pwd = validateDbPassword($_POST['pwd'], $driver);
+    $tableprefix = validateTablePrefix($_POST['tableprefix']);
+    $database_name = validateDbName($_POST['database_name']);
+    $database_collation = validateDbCollation($_POST['database_collation'] ?? '');
+} catch (InvalidArgumentException $e) {
+    exit($output . '<span id="database_fail">' . $_lang['status_failed'] . ' '
+        . htmlspecialchars($e->getMessage(), ENT_QUOTES, 'UTF-8') . '</span>');
+}
+$installMode = (int)$_POST['installMode'];
 
 $database_charset = getDatabaseCharset($database_collation, $driver);
 try {

install/src/controllers/install.php

@@ -100,7 +100,7 @@
         $confph['connection_collation'] = addslashes($database_collation);
         $confph['database_name'] = addslashes(
             $database_type === 'sqlite' ? sqliteDbNameToPath($database_name) : $database_name);
-        $confph['table_prefix'] = addslashes($_POST['tableprefix'] ?? '');
+        $confph['table_prefix'] = addslashes(validateTablePrefix($_POST['tableprefix'] ?? ''));
         $confph['lastInstallTime'] = time();
         $confph['site_sessionname'] = addslashes($site_sessionname);
         $confph['database_engine'] = '';

install/src/functions.php

@@ -258,19 +258,23 @@ function validateDbName($name)
     if (strlen($name) >= 64) {
         throw new InvalidArgumentException("Database name should be shorter than 64 characters");
     }
-    if (preg_match('/^[a-zA-Z][a-zA-Z0-9_-]{0,63}$/', $name) === false) {
-        throw new InvalidArgumentException("Database name should begin with letter and contain only letters,"
-            . " numbers, dashes, and underscores");
+    // The name is spliced into CREATE DATABASE and the config file. preg_match() returns 0, not
+    // false, for a mismatch, so the old "=== false" test let everything through.
+    if (preg_match('/^[A-Za-z0-9_$-]+$/', $name) !== 1) {
+        throw new InvalidArgumentException("Database name should contain only letters, numbers, dashes,"
+            . " dollar signs and underscores");
     }
     return $name;
 }
 
 function validateDbCollation($collation)
 {
     $collation = trim($collation ?? '');
-    if (preg_match('/^[a-zA-Z][a-zA-Z0-9_.-]{0,63}$/', $collation) === false) {
-        throw new InvalidArgumentException("Database name should begin with letter and contain only letters,"
-            . " numbers, dots, dashes, and underscores");
+    // Also spliced into CREATE DATABASE, unquoted on MySQL. "@" covers PostgreSQL locale
+    // modifiers such as sr_RS@latin.
+    if (preg_match('/^(?:[A-Za-z][A-Za-z0-9_.@-]{0,63})?$/', $collation) !== 1) {
+        throw new InvalidArgumentException("Database collation should begin with letter and contain only letters,"
+            . " numbers, dots, dashes, at signs and underscores");
     }
     return $collation;
 }
@@ -319,8 +323,8 @@ function validateDbHost($host, $type)
 
 function validateTablePrefix($prefix)
 {
-    $prefix = trim($prefix);
-    if (preg_match('/^[a-zA-Z0-9_]{0,40}_?$/', $prefix) === false) {
+    $prefix = trim((string) ($prefix ?? ''));
+    if (preg_match('/^[a-zA-Z0-9_]{0,40}_?$/', $prefix) !== 1) {
         throw new InvalidArgumentException('Invalid table prefix');
     }
     return $prefix;

install/src/lang.php

@@ -15,32 +15,39 @@
 #default fallback language file - english
 $install_language = 'en';
 
+// Language codes arrive from the request and are interpolated into include paths, so a code is
+// only accepted when it is a plain alphabetic stem that names a shipped locale file/directory.
+$installLanguageExists = static function ($code): bool {
+    return is_string($code) && ctype_alpha($code) && is_file(__DIR__ . '/lang/' . $code . '.inc.php');
+};
+$managerLanguageExists = static function ($code): bool {
+    return is_string($code) && ctype_alpha($code) && is_dir(dirname(__DIR__, 2) . '/core/lang/' . $code);
+};
+
 $_langISO6391 = substr($_SERVER['HTTP_ACCEPT_LANGUAGE'] ?? '', 0, 2);
-if (ctype_alpha($_langISO6391) && file_exists(__DIR__ . '/lang/' . $_langISO6391 . '.inc.php')) {
+if ($installLanguageExists($_langISO6391)) {
     $install_language = $_langISO6391;
 }
 
-if (isset($_POST['language']) && ctype_alpha($_POST['language'])) {
+if ($installLanguageExists($_POST['language'] ?? null)) {
     $install_language = $_POST['language'];
-} else {
-    if (isset($_GET['language']) && ctype_alpha($_GET['language'])) {
-        $install_language = $_GET['language'];
-    }
+} elseif ($installLanguageExists($_GET['language'] ?? null)) {
+    $install_language = $_GET['language'];
 }
 # load language file
-require_once 'lang/en.inc.php'; // As fallback
+require __DIR__ . '/lang/en.inc.php'; // As fallback
 $fallbackLang = $_lang;
-require_once 'lang/' . $install_language . '.inc.php';
+if ($install_language !== 'en') {
+    require __DIR__ . '/lang/' . $install_language . '.inc.php';
+}
 $_lang += $fallbackLang;
 
-$manager_language = $install_language;
+$manager_language = $managerLanguageExists($install_language) ? $install_language : 'en';
 
-if (isset($_POST['managerlanguage']) && ctype_alpha($_POST['managerlanguage'])) {
+if ($managerLanguageExists($_POST['managerlanguage'] ?? null)) {
     $manager_language = $_POST['managerlanguage'];
-} else {
-    if (isset($_GET['managerlanguage']) && ctype_alpha($_GET['managerlanguage'])) {
-        $manager_language = $_GET['managerlanguage'];
-    }
+} elseif ($managerLanguageExists($_GET['managerlanguage'] ?? null)) {
+    $manager_language = $_GET['managerlanguage'];
 }
 
 foreach ($_lang as $k => $v) {