- more injection

- less static calls - use params on sql queries - handle sql exception on database and user creation gracefully
2015-07-30 00:04:30 +02:00 · 2015-07-30 00:04:30 +02:00 · c3cac887f5
parent 114f128fc3
commit c3cac887f5
6 changed files with 118 additions and 66 deletions
--- 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) {
--- 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();
--- 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?');
 		}
--- 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,
--- 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);
+		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);
+			//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,
+		]);
+	}
 }
--- 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(