Page MenuHomePhabricator

Installer does not validate SQLite database directory for stable path
Closed, ResolvedPublic

Description

The SQLite database directory isn't being clearly validated; if you type in something like "foobar.sqlite" the installer will appear to complete but the site won't work -- you'll end up with a 'foobar.sqlite' subdirectory *inside* the 'config' folder, which you may not even have file permissions to move or rename.

The relative path of course then can't be reached from the main index.php running MediaWiki, and your site fails.

If a relative path is given, it should be rebased against $IP.


Version: unspecified
Severity: enhancement

Details

Reference
bz20244

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:49 PM
bzimport set Reference to bz20244.
bzimport added a subscriber: Unknown Object (MLST).

Listing the default in the installer might be good too. :) And we might need/want to expose wgSQLiteDataDirMode or set it more appropriately...

Max, would you mind looking at this?

I know we already expose the directory path in the new installer (the mode setting was useless and removed awhile ago)

As the matter of fact, we're already attempting to make the path absolute, but since we call realpath() before attempting to create the directory, it may fail. Here's my fix for it, I can't currently commit it myself.

Index: SqliteInstaller.php

  • SqliteInstaller.php (revision 77580)

+++ SqliteInstaller.php (working copy)
@@ -45,16 +45,30 @@

			$this->getTextBox( 'wgDBname', 'config-db-name', array(), $this->parent->getHelpBox( 'config-sqlite-name-help' ) );
	}

+ /*
+ * Safe wrapper for PHP's realpath() that fails gracefully if it's unable to canonicalize the path.
+ */
+ private static function realpath( $path ) {
+ $result = realpath( $path );
+ if ( !$result ) {
+ return $path;
+ }
+ return $result;
+ }
+

	public function submitConnectForm() {
		$this->setVarsFromRequest( array( 'wgSQLiteDataDir', 'wgDBname' ) );
  • $dir = realpath( $this->getVar( 'wgSQLiteDataDir' ) );
  • if ( !$dir ) {
  • // realpath() sometimes fails, especially on Windows
  • $dir = $this->getVar( 'wgSQLiteDataDir' );

+ # Try realpath() if the directory already exists
+ $dir = self::realpath( $this->getVar( 'wgSQLiteDataDir' ) );
+ $result = self::dataDirOKmaybeCreate( $dir, true /* create? */ );
+ if ( $result->isOK() )
+ {
+ # Try expanding again in case we've just created it
+ $dir = self::realpath( $dir );
+ $this->setVar( 'wgSQLiteDataDir', $dir );

		}
  • $this->setVar( 'wgSQLiteDataDir', $dir );
  • return self::dataDirOKmaybeCreate( $dir, true /* create? */ );

+ return $result;

	}

	private static function dataDirOKmaybeCreate( $dir, $create = false ) {

If the path can't be canonicalized, that sounds...... bad. What sort of failure are you referring to exactly, and why is it failing?

From PHP docs: "realpath() returns FALSE on failure, e.g. if the file does not exist."
I also observed it failing on Windows with paths containing spaces.

We just figured out the not-existing-yet one on IRC. :D I'll take a quick peek at the Windows case.