From c3cac887f57278a21052391c99b37a6dfb8cef9f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20M=C3=BCller?= <thomas.mueller@tmit.eu>
Date: Thu, 30 Jul 2015 00:04:30 +0200
Subject: [PATCH] - more injection - less static calls - use params on sql
queries - handle sql exception on database and user creation gracefully
---
core/command/maintenance/install.php | 5 +-
lib/base.php | 4 +-
lib/private/setup.php | 26 +++--
lib/private/setup/abstractdatabase.php | 12 ++-
lib/private/setup/mysql.php | 136 +++++++++++++++----------
lib/private/util.php | 3 +-
6 files changed, 119 insertions(+), 67 deletions(-)
diff --git a/core/command/maintenance/install.php b/core/command/maintenance/install.php
index 2fea5add438..7f5d9cae647 100644
--- a/core/command/maintenance/install.php
+++ b/core/command/maintenance/install.php
@@ -61,7 +61,10 @@ class Install extends Command {
protected function execute(InputInterface $input, OutputInterface $output) {
// validate the environment
- $setupHelper = new Setup($this->config, \OC::$server->getIniWrapper(), \OC::$server->getL10N('lib'), new \OC_Defaults());
+ $server = \OC::$server;
+ $setupHelper = new Setup($this->config, $server->getIniWrapper(),
+ $server->getL10N('lib'), new \OC_Defaults(), $server->getLogger(),
+ $server->getSecureRandom());
$sysInfo = $setupHelper->getSystemInfo(true);
$errors = $sysInfo['errors'];
if (count($errors) > 0) {
diff --git a/lib/base.php b/lib/base.php
index fde67839560..299970e269c 100644
--- a/lib/base.php
+++ b/lib/base.php
@@ -835,7 +835,9 @@ class OC {
// Check if ownCloud is installed or in maintenance (update) mode
if (!$systemConfig->getValue('installed', false)) {
\OC::$server->getSession()->clear();
- $setupHelper = new OC\Setup(\OC::$server->getConfig(), \OC::$server->getIniWrapper(), \OC::$server->getL10N('lib'), new \OC_Defaults());
+ $setupHelper = new OC\Setup(\OC::$server->getConfig(), \OC::$server->getIniWrapper(),
+ \OC::$server->getL10N('lib'), new \OC_Defaults(), \OC::$server->getLogger(),
+ \OC::$server->getSecureRandom());
$controller = new OC\Core\Setup\Controller($setupHelper);
$controller->run($_POST);
exit();
diff --git a/lib/private/setup.php b/lib/private/setup.php
index c862429fd2a..afc88256da4 100644
--- a/lib/private/setup.php
+++ b/lib/private/setup.php
@@ -39,6 +39,8 @@ use bantu\IniGetWrapper\IniGetWrapper;
use Exception;
use OCP\IConfig;
use OCP\IL10N;
+use OCP\ILogger;
+use OCP\Security\ISecureRandom;
class Setup {
/** @var \OCP\IConfig */
@@ -49,6 +51,10 @@ class Setup {
protected $l10n;
/** @var \OC_Defaults */
protected $defaults;
+ /** @var ILogger */
+ protected $logger;
+ /** @var ISecureRandom */
+ protected $random;
/**
* @param IConfig $config
@@ -58,11 +64,16 @@ class Setup {
function __construct(IConfig $config,
IniGetWrapper $iniWrapper,
IL10N $l10n,
- \OC_Defaults $defaults) {
+ \OC_Defaults $defaults,
+ ILogger $logger,
+ ISecureRandom $random
+ ) {
$this->config = $config;
$this->iniWrapper = $iniWrapper;
$this->l10n = $l10n;
$this->defaults = $defaults;
+ $this->logger = $logger;
+ $this->random = $random;
}
static $dbSetupClasses = array(
@@ -249,7 +260,8 @@ class Setup {
$class = self::$dbSetupClasses[$dbType];
/** @var \OC\Setup\AbstractDatabase $dbSetup */
- $dbSetup = new $class($l, 'db_structure.xml', $this->config);
+ $dbSetup = new $class($l, 'db_structure.xml', $this->config,
+ $this->logger, $this->random);
$error = array_merge($error, $dbSetup->validate($options));
// validate the data directory
@@ -284,9 +296,9 @@ class Setup {
}
//generate a random salt that is used to salt the local user passwords
- $salt = \OC::$server->getSecureRandom()->getLowStrengthGenerator()->generate(30);
+ $salt = $this->random->getLowStrengthGenerator()->generate(30);
// generate a secret
- $secret = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(48);
+ $secret = $this->random->getMediumStrengthGenerator()->generate(48);
//write the config file
$this->config->setSystemValues([
@@ -351,7 +363,7 @@ class Setup {
//try to write logtimezone
if (date_default_timezone_get()) {
- \OC_Config::setValue('logtimezone', date_default_timezone_get());
+ $config->setSystemValue('logtimezone', date_default_timezone_get());
}
//and we are done
@@ -389,7 +401,9 @@ class Setup {
* @throws \OC\HintException If .htaccess does not include the current version
*/
public static function updateHtaccess() {
- $setupHelper = new \OC\Setup(\OC::$server->getConfig(), \OC::$server->getIniWrapper(), \OC::$server->getL10N('lib'), new \OC_Defaults());
+ $setupHelper = new \OC\Setup(\OC::$server->getConfig(), \OC::$server->getIniWrapper(),
+ \OC::$server->getL10N('lib'), new \OC_Defaults(), \OC::$server->getLogger(),
+ \OC::$server->getSecureRandom());
if(!$setupHelper->isCurrentHtaccess()) {
throw new \OC\HintException('.htaccess file has the wrong version. Please upload the correct version. Maybe you forgot to replace it after updating?');
}
diff --git a/lib/private/setup/abstractdatabase.php b/lib/private/setup/abstractdatabase.php
index 928af0568b5..1ec853c3b02 100644
--- a/lib/private/setup/abstractdatabase.php
+++ b/lib/private/setup/abstractdatabase.php
@@ -23,6 +23,8 @@
namespace OC\Setup;
use OCP\IConfig;
+use OCP\ILogger;
+use OCP\Security\ISecureRandom;
abstract class AbstractDatabase {
@@ -42,11 +44,17 @@ abstract class AbstractDatabase {
protected $tablePrefix;
/** @var IConfig */
protected $config;
+ /** @var ILogger */
+ protected $logger;
+ /** @var ISecureRandom */
+ protected $random;
- public function __construct($trans, $dbDefinitionFile, IConfig $config) {
+ public function __construct($trans, $dbDefinitionFile, IConfig $config, ILogger $logger, ISecureRandom $random) {
$this->trans = $trans;
$this->dbDefinitionFile = $dbDefinitionFile;
$this->config = $config;
+ $this->logger = $logger;
+ $this->random = $random;
}
public function validate($config) {
@@ -70,7 +78,7 @@ abstract class AbstractDatabase {
$dbHost = !empty($config['dbhost']) ? $config['dbhost'] : 'localhost';
$dbTablePrefix = isset($config['dbtableprefix']) ? $config['dbtableprefix'] : 'oc_';
- \OC_Config::setValues([
+ $this->config->setSystemValues([
'dbname' => $dbName,
'dbhost' => $dbHost,
'dbtableprefix' => $dbTablePrefix,
diff --git a/lib/private/setup/mysql.php b/lib/private/setup/mysql.php
index 906b9f1b6c5..5597592f21e 100644
--- a/lib/private/setup/mysql.php
+++ b/lib/private/setup/mysql.php
@@ -24,6 +24,7 @@
namespace OC\Setup;
use OC\DB\ConnectionFactory;
+use OCP\IDBConnection;
class MySQL extends AbstractDatabase {
public $dbprettyname = 'MySQL/MariaDB';
@@ -31,58 +32,15 @@ class MySQL extends AbstractDatabase {
public function setupDatabase($username) {
//check if the database user has admin right
$connection = $this->connect();
- //user already specified in config
- $oldUser=\OC_Config::getValue('dbuser', false);
- //we don't have a dbuser specified in config
- if($this->dbUser!=$oldUser) {
- //add prefix to the admin username to prevent collisions
- $adminUser=substr('oc_'.$username, 0, 16);
-
- $i = 1;
- while(true) {
- //this should be enough to check for admin rights in mysql
- $query="SELECT user FROM mysql.user WHERE user='$adminUser'";
- $result = $connection->executeQuery($query);
-
- //current dbuser has admin rights
- if($result) {
- $data = $result->fetchAll();
- //new dbuser does not exist
- if(count($data) === 0) {
- //use the admin login data for the new database user
- $this->dbUser=$adminUser;
-
- //create a random password so we don't need to store the admin password in the config file
- $this->dbPassword=\OC_Util::generateRandomBytes(30);
-
- $this->createDBUser($connection);
-
- break;
- } else {
- //repeat with different username
- $length=strlen((string)$i);
- $adminUser=substr('oc_'.$username, 0, 16 - $length).$i;
- $i++;
- }
- } else {
- break;
- }
- };
-
- \OC_Config::setValues([
- 'dbuser' => $this->dbUser,
- 'dbpassword' => $this->dbPassword,
- ]);
- }
+ $this->createSpecificUser($username, $connection);
//create the database
$this->createDatabase($connection);
//fill the database if needed
- $query='select count(*) from information_schema.tables'
- ." where table_schema='".$this->dbName."' AND table_name = '".$this->tablePrefix."users';";
- $result = $connection->executeQuery($query);
+ $query='select count(*) from information_schema.tables where table_schema=? AND table_name = ?';
+ $result = $connection->executeQuery($query, [$this->dbName, $this->tablePrefix.'users']);
$row = $result->fetch();
if(!$result or $row[0]==0) {
\OC_DB::createDbFromStructure($this->dbDefinitionFile);
@@ -93,19 +51,26 @@ class MySQL extends AbstractDatabase {
* @param \OC\DB\Connection $connection
*/
private function createDatabase($connection) {
- $name = $this->dbName;
- $user = $this->dbUser;
- //we cant use OC_BD functions here because we need to connect as the administrative user.
- $query = "CREATE DATABASE IF NOT EXISTS `$name` CHARACTER SET utf8 COLLATE utf8_bin;";
- $connection->executeUpdate($query);
-
- //this query will fail if there aren't the right permissions, ignore the error
- $query="GRANT ALL PRIVILEGES ON `$name` . * TO '$user'";
- $connection->executeUpdate($query);
+ try{
+ $name = $this->dbName;
+ $user = $this->dbUser;
+ //we cant use OC_BD functions here because we need to connect as the administrative user.
+ $query = "CREATE DATABASE IF NOT EXISTS `$name` CHARACTER SET utf8 COLLATE utf8_bin;";
+ $connection->executeUpdate($query);
+
+ //this query will fail if there aren't the right permissions, ignore the error
+ $query="GRANT ALL PRIVILEGES ON `$name` . * TO '$user'";
+ $connection->executeUpdate($query);
+ } catch (\Exception $ex) {
+ $this->logger->error('Database creation failed: {error}', [
+ 'app' => 'mysql.setup',
+ 'error' => $ex->getMessage()
+ ]);
+ }
}
/**
- * @param \OC\DB\Connection $connection
+ * @param IDbConnection $connection
* @throws \OC\DatabaseSetupException
*/
private function createDBUser($connection) {
@@ -134,4 +99,63 @@ class MySQL extends AbstractDatabase {
$cf = new ConnectionFactory();
return $cf->getConnection($type, $connectionParams);
}
+
+ /**
+ * @param $username
+ * @param IDBConnection $connection
+ * @return array
+ */
+ private function createSpecificUser($username, $connection) {
+ try {
+ //user already specified in config
+ $oldUser = $this->config->getSystemValue('dbuser', false);
+
+ //we don't have a dbuser specified in config
+ if ($this->dbUser !== $oldUser) {
+ //add prefix to the admin username to prevent collisions
+ $adminUser = substr('oc_' . $username, 0, 16);
+
+ $i = 1;
+ while (true) {
+ //this should be enough to check for admin rights in mysql
+ $query = 'SELECT user FROM mysql.user WHERE user=?';
+ $result = $connection->executeQuery($query, [$adminUser]);
+
+ //current dbuser has admin rights
+ if ($result) {
+ $data = $result->fetchAll();
+ //new dbuser does not exist
+ if (count($data) === 0) {
+ //use the admin login data for the new database user
+ $this->dbUser = $adminUser;
+
+ //create a random password so we don't need to store the admin password in the config file
+ $this->dbPassword = $this->random->getMediumStrengthGenerator()->generate(30);
+
+ $this->createDBUser($connection);
+
+ break;
+ } else {
+ //repeat with different username
+ $length = strlen((string)$i);
+ $adminUser = substr('oc_' . $username, 0, 16 - $length) . $i;
+ $i++;
+ }
+ } else {
+ break;
+ }
+ };
+ }
+ } catch (\Exception $ex) {
+ $this->logger->error('Specific user creation failed: {error}', [
+ 'app' => 'mysql.setup',
+ 'error' => $ex->getMessage()
+ ]);
+ }
+
+ $this->config->setSystemValues([
+ 'dbuser' => $this->dbUser,
+ 'dbpassword' => $this->dbPassword,
+ ]);
+ }
}
diff --git a/lib/private/util.php b/lib/private/util.php
index 39d64952dc6..1b22e03ca6f 100644
--- a/lib/private/util.php
+++ b/lib/private/util.php
@@ -567,7 +567,8 @@ class OC_Util {
}
$webServerRestart = false;
- $setup = new \OC\Setup($config, \OC::$server->getIniWrapper(), \OC::$server->getL10N('lib'), new \OC_Defaults());
+ $setup = new \OC\Setup($config, \OC::$server->getIniWrapper(), \OC::$server->getL10N('lib'),
+ new \OC_Defaults(), \OC::$server->getLogger(), \OC::$server->getSecureRandom());
$availableDatabases = $setup->getSupportedDatabases();
if (empty($availableDatabases)) {
$errors[] = array(
--
GitLab